devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC
@ 2017-09-17  8:23 Marty E. Plummer
  2017-09-17  8:23 ` [RFC RESEND 1/3] clk: hisilicon: add CRG driver " Marty E. Plummer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Marty E. Plummer @ 2017-09-17  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, xuejiancheng, leo.yan, linux-clk, linux-kernel,
	mark.rutland, mturquette, wenpan, robh+dt, linux, sboyd, xuwei5,
	zhangfei.gao, gregkh, arnd, Marty E. Plummer

Greetings,

I'd like the community's feedback on the following patchset. I've attempted to
split my changes up in what I believe to be a sensible setup.

The device I'm working against is the 'SamsungSV SDR-B74301' HD CCTV surveillance
system, which uses a Hisilicon Hi3521A arm SoC as its basis.

Resending due to a typo, s/primcell/primecell/

Marty E. Plummer (3):
  clk: hisilicon: add CRG driver Hi3521A SoC
  arm: hisi: enable Hi3521A SoC
  arm: dts: add Hi3521A dts

 arch/arm/boot/dts/Makefile                |   2 +
 arch/arm/boot/dts/hi3521a-rs-dm290e.dts   |  52 +++++
 arch/arm/boot/dts/hi3521a.dtsi            | 310 ++++++++++++++++++++++++++++++
 arch/arm/mach-hisi/Kconfig                |   6 +
 drivers/clk/hisilicon/Kconfig             |   7 +
 drivers/clk/hisilicon/Makefile            |   1 +
 drivers/clk/hisilicon/crg-hi3521a.c       | 207 ++++++++++++++++++++
 include/dt-bindings/clock/hi3521a-clock.h |  34 ++++
 8 files changed, 619 insertions(+)
 create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
 create mode 100644 arch/arm/boot/dts/hi3521a.dtsi
 create mode 100644 drivers/clk/hisilicon/crg-hi3521a.c
 create mode 100644 include/dt-bindings/clock/hi3521a-clock.h

-- 
2.14.1


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

* [RFC RESEND 1/3] clk: hisilicon: add CRG driver Hi3521A SoC
  2017-09-17  8:23 [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Marty E. Plummer
@ 2017-09-17  8:23 ` Marty E. Plummer
  2017-09-17  8:23 ` [RFC RESEND 2/3] arm: hisi: enable " Marty E. Plummer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Marty E. Plummer @ 2017-09-17  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, xuejiancheng, leo.yan, linux-clk, linux-kernel,
	mark.rutland, mturquette, wenpan, robh+dt, linux, sboyd, xuwei5,
	zhangfei.gao, gregkh, arnd, Marty E. Plummer

Add CRG driver for Hi3521A SoC. CRG (Clock and Reset Generator) module
generates clock and reset signals used by other module blocks on SoC.

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
---
 drivers/clk/hisilicon/Kconfig             |   7 +
 drivers/clk/hisilicon/Makefile            |   1 +
 drivers/clk/hisilicon/crg-hi3521a.c       | 207 ++++++++++++++++++++++++++++++
 include/dt-bindings/clock/hi3521a-clock.h |  34 +++++
 4 files changed, 249 insertions(+)
 create mode 100644 drivers/clk/hisilicon/crg-hi3521a.c
 create mode 100644 include/dt-bindings/clock/hi3521a-clock.h

diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig
index 7098bfd32b1b..d93e3180a04f 100644
--- a/drivers/clk/hisilicon/Kconfig
+++ b/drivers/clk/hisilicon/Kconfig
@@ -14,6 +14,13 @@ config COMMON_CLK_HI3519
 	help
 	  Build the clock driver for hi3519.
 
+config COMMON_CLK_HI3521A
+	bool "Hi3521A/Hi3520DV300 Clock Driver"
+	depends on ARCH_HISI || COMPILE_TEST
+	default ARCH_HISI
+	help
+	  Build the clock driver for hi3521a/hi3520dv300
+
 config COMMON_CLK_HI3660
 	bool "Hi3660 Clock Driver"
 	depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
index 1e4c3ddbad84..46f8d619c923 100644
--- a/drivers/clk/hisilicon/Makefile
+++ b/drivers/clk/hisilicon/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_ARCH_HIP04)	+= clk-hip04.o
 obj-$(CONFIG_ARCH_HIX5HD2)	+= clk-hix5hd2.o
 obj-$(CONFIG_COMMON_CLK_HI3516CV300)	+= crg-hi3516cv300.o
 obj-$(CONFIG_COMMON_CLK_HI3519)	+= clk-hi3519.o
+obj-$(CONFIG_COMMON_CLK_HI3521A)+= crg-hi3521a.o
 obj-$(CONFIG_COMMON_CLK_HI3660) += clk-hi3660.o
 obj-$(CONFIG_COMMON_CLK_HI3798CV200)	+= crg-hi3798cv200.o
 obj-$(CONFIG_COMMON_CLK_HI6220)	+= clk-hi6220.o
diff --git a/drivers/clk/hisilicon/crg-hi3521a.c b/drivers/clk/hisilicon/crg-hi3521a.c
new file mode 100644
index 000000000000..8c03ce2694c1
--- /dev/null
+++ b/drivers/clk/hisilicon/crg-hi3521a.c
@@ -0,0 +1,207 @@
+/*
+ * Copyright (C) 2017 Marty E. Plummer <hanetzer@startmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <dt-bindings/clock/hi3521a-clock.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include "clk.h"
+#include "reset.h"
+
+#define HI3521A_INNER_CLK_OFFSET	64
+#define HI3521A_FIXED_2M		65
+#define HI3521A_FIXED_24M		66
+#define HI3521A_FIXED_50M		67
+#define HI3521A_FIXED_83M		68
+#define HI3521A_FIXED_100M		69
+#define HI3521A_FIXED_150M		70
+#define HI3521A_FMC_MUX			71
+#define HI3521A_UART_MUX		72
+
+#define HI3521A_NR_CLKS			128
+
+struct hi3521a_crg_data {
+	struct hisi_clock_data *clk_data;
+	struct hisi_reset_controller *rstc;
+};
+
+static const struct hisi_fixed_rate_clock hi3521a_fixed_rate_clks[] = {
+	{ HI3521A_FIXED_2M,     "2m", NULL, 0,   2000000, },
+	{ HI3521A_FIXED_24M,   "24m", NULL, 0,  24000000, },
+	{ HI3521A_FIXED_50M,   "50m", NULL, 0,  50000000, },
+	{ HI3521A_FIXED_83M,   "83m", NULL, 0,  83000000, },
+	{ HI3521A_FIXED_100M, "100m", NULL, 0, 100000000, },
+	{ HI3521A_FIXED_150M, "150m", NULL, 0, 150000000, },
+};
+
+static const char *const uart_mux_p[] = { "50m", "2m", "24m", };
+static const char *const fmc_mux_p[] = { "24m", "83m", "150m", };
+
+static u32 uart_mux_table[] = {0, 1, 2};
+static u32 fmc_mux_table[] = {0, 1, 2};
+
+static const struct hisi_mux_clock hi3521a_mux_clks[] = {
+	{ HI3521A_UART_MUX, "uart_mux", uart_mux_p, ARRAY_SIZE(uart_mux_p),
+		CLK_SET_RATE_PARENT, 0x84, 18, 2, 0, uart_mux_table, },
+	{ HI3521A_FMC_MUX, "fmc_mux", fmc_mux_p, ARRAY_SIZE(fmc_mux_p),
+		CLK_SET_RATE_PARENT, 0x74, 2, 2, 0, fmc_mux_table, },
+};
+
+static const struct hisi_gate_clock hi3521a_gate_clks[] = {
+	{ HI3521A_FMC_CLK, "clk_fmc", "fmc_mux", CLK_SET_RATE_PARENT,
+		0x74, 1, 0, },
+	{ HI3521A_UART0_CLK, "clk_uart0", "uart_mux", CLK_SET_RATE_PARENT,
+		0x84, 15, 0, },
+	{ HI3521A_UART1_CLK, "clk_uart1", "uart_mux", CLK_SET_RATE_PARENT,
+		0x84, 16, 0, },
+	{ HI3521A_UART2_CLK, "clk_uart2", "uart_mux", CLK_SET_RATE_PARENT,
+		0x84, 17, 0, },
+	{ HI3521A_SPI0_CLK, "clk_spi0", "50m", CLK_SET_RATE_PARENT,
+		0x84, 13, 0, },
+	/* { HI3521A_ETH_CLK, "clk_eth", NULL, */
+	/* 	0, 0x78, 1, 0, }, */
+	/* { HI3521A_ETH_MACIF_CLK, "clk_eth_macif", NULL, */
+	/* 	0, 0x78, 3, 0 }, */
+};
+
+static struct hisi_clock_data *hi3521a_clk_register(struct platform_device *pdev)
+{
+	struct hisi_clock_data *clk_data;
+	int ret;
+
+	clk_data = hisi_clk_alloc(pdev, HI3521A_NR_CLKS);
+	if (!clk_data)
+		return ERR_PTR(-ENOMEM);
+
+	ret = hisi_clk_register_fixed_rate(hi3521a_fixed_rate_clks,
+				     ARRAY_SIZE(hi3521a_fixed_rate_clks),
+				     clk_data);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = hisi_clk_register_mux(hi3521a_mux_clks,
+				ARRAY_SIZE(hi3521a_mux_clks),
+				clk_data);
+	if (ret)
+		goto unregister_fixed_rate;
+
+	ret = hisi_clk_register_gate(hi3521a_gate_clks,
+				ARRAY_SIZE(hi3521a_gate_clks),
+				clk_data);
+	if (ret)
+		goto unregister_mux;
+
+	ret = of_clk_add_provider(pdev->dev.of_node,
+			of_clk_src_onecell_get, &clk_data->clk_data);
+	if (ret)
+		goto unregister_gate;
+
+	return clk_data;
+
+unregister_fixed_rate:
+	hisi_clk_unregister_fixed_rate(hi3521a_fixed_rate_clks,
+				ARRAY_SIZE(hi3521a_fixed_rate_clks),
+				clk_data);
+
+unregister_mux:
+	hisi_clk_unregister_mux(hi3521a_mux_clks,
+				ARRAY_SIZE(hi3521a_mux_clks),
+				clk_data);
+unregister_gate:
+	hisi_clk_unregister_gate(hi3521a_gate_clks,
+				ARRAY_SIZE(hi3521a_gate_clks),
+				clk_data);
+	return ERR_PTR(ret);
+}
+
+static void hi3521a_clk_unregister(struct platform_device *pdev)
+{
+	struct hi3521a_crg_data *crg = platform_get_drvdata(pdev);
+
+	of_clk_del_provider(pdev->dev.of_node);
+
+	hisi_clk_unregister_gate(hi3521a_gate_clks,
+				ARRAY_SIZE(hi3521a_mux_clks),
+				crg->clk_data);
+	hisi_clk_unregister_mux(hi3521a_mux_clks,
+				ARRAY_SIZE(hi3521a_mux_clks),
+				crg->clk_data);
+	hisi_clk_unregister_fixed_rate(hi3521a_fixed_rate_clks,
+				ARRAY_SIZE(hi3521a_fixed_rate_clks),
+				crg->clk_data);
+}
+
+static int hi3521a_clk_probe(struct platform_device *pdev)
+{
+	struct hi3521a_crg_data *crg;
+
+	crg = devm_kmalloc(&pdev->dev, sizeof(*crg), GFP_KERNEL);
+	if (!crg)
+		return -ENOMEM;
+
+	crg->rstc = hisi_reset_init(pdev);
+	if (!crg->rstc)
+		return -ENOMEM;
+
+	crg->clk_data = hi3521a_clk_register(pdev);
+	if (IS_ERR(crg->clk_data)) {
+		hisi_reset_exit(crg->rstc);
+		return PTR_ERR(crg->clk_data);
+	}
+
+	platform_set_drvdata(pdev, crg);
+	return 0;
+}
+
+static int hi3521a_clk_remove(struct platform_device *pdev)
+{
+	struct hi3521a_crg_data *crg = platform_get_drvdata(pdev);
+
+	hisi_reset_exit(crg->rstc);
+	hi3521a_clk_unregister(pdev);
+	return 0;
+}
+
+static const struct of_device_id hi3521a_clk_match_table[] = {
+	{ .compatible = "hisilicon,hi3521a-crg" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, hi3521a_clk_match_table);
+
+static struct platform_driver hi3521a_clk_driver = {
+	.probe		= hi3521a_clk_probe,
+	.remove		= hi3521a_clk_remove,
+	.driver		= {
+		.name	= "hi3521a-clk",
+		.of_match_table = hi3521a_clk_match_table,
+	},
+};
+
+static int __init hi3521a_clk_init(void)
+{
+	return platform_driver_register(&hi3521a_clk_driver);
+}
+core_initcall(hi3521a_clk_init);
+
+static void __exit hi3521a_clk_exit(void)
+{
+	platform_driver_unregister(&hi3521a_clk_driver);
+}
+module_exit(hi3521a_clk_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("HiSilicon Hi3521a Clock Driver");
diff --git a/include/dt-bindings/clock/hi3521a-clock.h b/include/dt-bindings/clock/hi3521a-clock.h
new file mode 100644
index 000000000000..666591236169
--- /dev/null
+++ b/include/dt-bindings/clock/hi3521a-clock.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2017 Marty E. Plummer <hanetzer@startmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __DTS_HI3521A_CLOCK_H
+#define __DTS_HI3521A_CLOCK_H
+
+#define HI3521A_FMC_CLK			1
+#define HI3521A_SPI0_CLK		2
+#define HI3521A_UART0_CLK		3
+#define HI3521A_UART1_CLK		4
+#define HI3521A_UART2_CLK		5
+#define HI3521A_DMA_CLK			6
+#define HI3521A_IR_CLK			7
+#define HI3521A_ETH_PHY_CLK		8
+#define HI3521A_ETH_CLK			9
+#define HI3521A_ETH_MACIF_CLK		10
+#define HI3521A_USB2_BUS_CLK		11
+#define HI3521A_USB2_PORT_CLK		12
+
+#endif /* __DTS_HI3521A_CLK_H */
-- 
2.14.1


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

* [RFC RESEND 2/3] arm: hisi: enable Hi3521A SoC
  2017-09-17  8:23 [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Marty E. Plummer
  2017-09-17  8:23 ` [RFC RESEND 1/3] clk: hisilicon: add CRG driver " Marty E. Plummer
@ 2017-09-17  8:23 ` Marty E. Plummer
  2017-09-17  8:23 ` [RFC RESEND 3/3] arm: dts: add Hi3521A dts Marty E. Plummer
  2017-09-18 10:55 ` [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Greg KH
  3 siblings, 0 replies; 9+ messages in thread
From: Marty E. Plummer @ 2017-09-17  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, xuejiancheng, leo.yan, linux-clk, linux-kernel,
	mark.rutland, mturquette, wenpan, robh+dt, linux, sboyd, xuwei5,
	zhangfei.gao, gregkh, arnd, Marty E. Plummer

Enable Hisilicon Hi3521A/Hi3520DCV300 SoC. This SoC series includes
hardware mutlimedia codec cores, commonly used in consumer cctv/dvr
security systems and ipcameras. The arm core is a Cortex A7.

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
---
 arch/arm/mach-hisi/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index 65a048fa08ec..26755414f862 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -12,6 +12,12 @@ if ARCH_HISI
 
 menu "Hisilicon platform type"
 
+config ARCH_HI3521A
+	bool "Hisilicon Hi3521A/Hi3520DCV300 family"
+	depends on ARCH_MULTI_V7
+	help
+	  Support for Hisilicon Hi3521A/Hi3520DCV300 SoC family
+
 config ARCH_HI3xxx
 	bool "Hisilicon Hi36xx family"
 	depends on ARCH_MULTI_V7
-- 
2.14.1


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

* [RFC RESEND 3/3] arm: dts: add Hi3521A dts
  2017-09-17  8:23 [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Marty E. Plummer
  2017-09-17  8:23 ` [RFC RESEND 1/3] clk: hisilicon: add CRG driver " Marty E. Plummer
  2017-09-17  8:23 ` [RFC RESEND 2/3] arm: hisi: enable " Marty E. Plummer
@ 2017-09-17  8:23 ` Marty E. Plummer
       [not found]   ` <20170917082327.10058-4-hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
  2017-09-18 10:55 ` [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Greg KH
  3 siblings, 1 reply; 9+ messages in thread
From: Marty E. Plummer @ 2017-09-17  8:23 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, xuejiancheng, leo.yan, linux-clk, linux-kernel,
	mark.rutland, mturquette, wenpan, robh+dt, linux, sboyd, xuwei5,
	zhangfei.gao, gregkh, arnd, Marty E. Plummer

Add hi3521a.dtsi and hi3521a-rs-dm290e.dts for RaySharp CCTV systems,
marketed under the name Samsung SDR-B74301N

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
---
 arch/arm/boot/dts/Makefile              |   2 +
 arch/arm/boot/dts/hi3521a-rs-dm290e.dts |  52 ++++++
 arch/arm/boot/dts/hi3521a.dtsi          | 310 ++++++++++++++++++++++++++++++++
 3 files changed, 364 insertions(+)
 create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
 create mode 100644 arch/arm/boot/dts/hi3521a.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index faf46abaa4a2..e7b9b5dde20f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -189,6 +189,8 @@ dtb-$(CONFIG_ARCH_GEMINI) += \
 	gemini-sq201.dtb \
 	gemini-wbd111.dtb \
 	gemini-wbd222.dtb
+dtb-$(CONFIG_ARCH_HI3521A) += \
+	hi3521a-rs-dm290e.dtb
 dtb-$(CONFIG_ARCH_HI3xxx) += \
 	hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += \
diff --git a/arch/arm/boot/dts/hi3521a-rs-dm290e.dts b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
new file mode 100644
index 000000000000..b32c8392c93f
--- /dev/null
+++ b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/dts-v1/;
+#include "hi3521a.dtsi"
+
+/ {
+	model = "RaySharp RS-DM-290E DVR Board";
+	compatible = "hisilicon,hi3521a";
+
+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x80000000 0xf00000>;
+	};
+};
+
+&hi_sfc {
+	status = "okay";
+	spi-nor@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <104000000>;
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&dual_timer0 {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/hi3521a.dtsi b/arch/arm/boot/dts/hi3521a.dtsi
new file mode 100644
index 000000000000..2af746fdec46
--- /dev/null
+++ b/arch/arm/boot/dts/hi3521a.dtsi
@@ -0,0 +1,310 @@
+/*
+ * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <dt-bindings/clock/hi3521a-clock.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	chosen { };
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a7";
+			reg = <0>;
+		};
+	};
+
+	hi_sfc: spi-nor-controller@10000000 {
+		compatible = "hisilicon,hi3521a-spi-nor", "hisilicon,fmc-spi-nor";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10000000 0x10000>, <0x14000000 0x1000000>;
+		reg-names = "control", "memory";
+		clocks = <&crg HI3521A_FMC_CLK>;
+		status = "disabled";
+	};
+
+	gic: interrupt-controller@10300000 {
+		compatible = "arm,pl390";
+		#interrupt-cells = <3>;
+		interrupt-controller;
+		reg = <0x10301000 0x1000>, <0x10302000 0x1000>;
+	};
+
+	clk_3m: clk_3m {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <3000000>;
+	};
+
+	crg: clock-reset-controller@12040000 {
+		compatible = "hisilicon,hi3521a-crg";
+		#clock-cells = <1>;
+		#reset-cells = <2>;
+		reg = <0x12040000 0x10000>;
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&gic>;
+		ranges;
+
+		dmac: dma@10060000 {
+			compatible = "arm,pl080", "arm,primecell";
+			reg = <0x10060000 0x1000>;
+			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		dual_timer0: timer@12000000 {
+			compatible = "arm,sp804", "arm,primecell";
+			interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x12000000 0x1000>;
+			clocks = <&clk_3m>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		dual_timer1: timer@12010000 {
+			compatible = "arm,sp804", "arm,primecell";
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x12010000 0x1000>;
+			clocks = <&clk_3m>;
+			clock-name = "apb_pclk";
+			status = "disabled";
+		};
+
+		dual_timer2: timer@12020000 {
+			compatible = "arm,sp804", "arm,primecell";
+			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x12020000 0x1000>;
+			clocks = <&clk_3m>;
+			clock-name = "apb_pclk";
+			status = "disabled";
+		};
+
+		dual_timer3: timer@12030000 {
+			compatible = "arm,sp804", "arm,primecell";
+			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x12030000 0x1000>;
+			clocks = <&clk_3m>;
+			clock-name = "apb_pclk";
+			status = "disabled";
+		};
+
+		wdt0: watchdog@12070000 {
+			compatible = "arm,sp805", "arm,primecell";
+			arm,primecell-periphid = <0x00141805>;
+			reg = <0x12070000 0x1000>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk_3m>;
+			clock-names = "apb_pclk";
+		};
+
+		uart0: serial@12080000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x12080000 0x1000>;
+			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HI3521A_UART0_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart1: serial@12090000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x12090000 0x1000>;
+			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HI3521A_UART1_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart2: serial@120a0000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x120a0000 0x1000>;
+			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HI3521A_UART2_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio0: gpio@12150000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12150000 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio1: gpio@12160000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12160000 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio2: gpio@12170000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12170000 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio3: gpio@12180000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12180000 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio4: gpio@12190000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12190000 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio5: gpio@121a0000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x121a0000 0x1000>;
+			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio6: gpio@121b0000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x121b0000 0x1000>;
+			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio7: gpio@121c0000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x121c0000 0x1000>;
+			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio8: gpio@121d0000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x121d0000 0x1000>;
+			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio9: gpio@121e0000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x121e0000 0x1000>;
+			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio10: gpio@121f0000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x121f0000 0x1000>;
+			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio11: gpio@12200000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12200000 0x1000>;
+			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio12: gpio@12210000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12210000 0x1000>;
+			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+
+		gpio13: gpio@12220000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x12220000 0x1000>;
+			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			status = "disabled";
+		};
+	};
+};
-- 
2.14.1


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

* Re: [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC
  2017-09-17  8:23 [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Marty E. Plummer
                   ` (2 preceding siblings ...)
  2017-09-17  8:23 ` [RFC RESEND 3/3] arm: dts: add Hi3521A dts Marty E. Plummer
@ 2017-09-18 10:55 ` Greg KH
  3 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2017-09-18 10:55 UTC (permalink / raw)
  To: Marty E. Plummer
  Cc: linux-arm-kernel, devicetree, xuejiancheng, leo.yan, linux-clk,
	linux-kernel, mark.rutland, mturquette, wenpan, robh+dt, linux,
	sboyd, xuwei5, zhangfei.gao, arnd

On Sun, Sep 17, 2017 at 03:23:24AM -0500, Marty E. Plummer wrote:
> Greetings,
> 
> I'd like the community's feedback on the following patchset. I've attempted to
> split my changes up in what I believe to be a sensible setup.
> 
> The device I'm working against is the 'SamsungSV SDR-B74301' HD CCTV surveillance
> system, which uses a Hisilicon Hi3521A arm SoC as its basis.
> 
> Resending due to a typo, s/primcell/primecell/
> 
> Marty E. Plummer (3):
>   clk: hisilicon: add CRG driver Hi3521A SoC
>   arm: hisi: enable Hi3521A SoC
>   arm: dts: add Hi3521A dts

Seems reasonable to me, but I'll let the clk maintainers/developers give
you a better review...

thanks,

greg k-h

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

* Re: [RFC RESEND 3/3] arm: dts: add Hi3521A dts
       [not found]   ` <20170917082327.10058-4-hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
@ 2017-09-20 20:53     ` Rob Herring
  2017-09-20 23:04       ` Marty E. Plummer
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2017-09-20 20:53 UTC (permalink / raw)
  To: Marty E. Plummer
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
	leo.yan-QSEj5FYQhm4dnm+yROfE0A, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
	mturquette-rdvid1DuHRBWk0Htik3J/w, wenpan-C8/M+/jPZTeaMJb+Lgu22Q,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
	zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, arnd-r2nGTMty4D4

On Sun, Sep 17, 2017 at 03:23:27AM -0500, Marty E. Plummer wrote:
> Add hi3521a.dtsi and hi3521a-rs-dm290e.dts for RaySharp CCTV systems,
> marketed under the name Samsung SDR-B74301N
> 
> Signed-off-by: Marty E. Plummer <hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile              |   2 +
>  arch/arm/boot/dts/hi3521a-rs-dm290e.dts |  52 ++++++
>  arch/arm/boot/dts/hi3521a.dtsi          | 310 ++++++++++++++++++++++++++++++++
>  3 files changed, 364 insertions(+)
>  create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
>  create mode 100644 arch/arm/boot/dts/hi3521a.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index faf46abaa4a2..e7b9b5dde20f 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -189,6 +189,8 @@ dtb-$(CONFIG_ARCH_GEMINI) += \
>  	gemini-sq201.dtb \
>  	gemini-wbd111.dtb \
>  	gemini-wbd222.dtb
> +dtb-$(CONFIG_ARCH_HI3521A) += \
> +	hi3521a-rs-dm290e.dtb
>  dtb-$(CONFIG_ARCH_HI3xxx) += \
>  	hi3620-hi4511.dtb
>  dtb-$(CONFIG_ARCH_HIGHBANK) += \
> diff --git a/arch/arm/boot/dts/hi3521a-rs-dm290e.dts b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> new file mode 100644
> index 000000000000..b32c8392c93f
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright (C) 2017 Marty Plummer <hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.

Should be version 2 or later? Doesn't really matter to me from a DT 
perspective, but it is in the kernel tree.

You can use SPDX tags if you want.

> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/dts-v1/;
> +#include "hi3521a.dtsi"
> +
> +/ {
> +	model = "RaySharp RS-DM-290E DVR Board";
> +	compatible = "hisilicon,hi3521a";

Needs a board compatible too.

> +
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +	};
> +
> +	memory {

Needs a unit-address.

> +		device_type = "memory";
> +		reg = <0x80000000 0xf00000>;
> +	};
> +};
> +
> +&hi_sfc {
> +	status = "okay";
> +	spi-nor@0 {
> +		compatible = "jedec,spi-nor";

I don't remember offhand, but I think this should have a device specific 
compatible too.

> +		reg = <0>;
> +		spi-max-frequency = <104000000>;
> +	};
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&dual_timer0 {
> +	status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/hi3521a.dtsi b/arch/arm/boot/dts/hi3521a.dtsi
> new file mode 100644
> index 000000000000..2af746fdec46
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3521a.dtsi
> @@ -0,0 +1,310 @@
> +/*
> + * Copyright (C) 2017 Marty Plummer <hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <dt-bindings/clock/hi3521a-clock.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	chosen { };
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0>;
> +		};
> +	};
> +
> +	hi_sfc: spi-nor-controller@10000000 {
> +		compatible = "hisilicon,hi3521a-spi-nor", "hisilicon,fmc-spi-nor";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x10000000 0x10000>, <0x14000000 0x1000000>;
> +		reg-names = "control", "memory";
> +		clocks = <&crg HI3521A_FMC_CLK>;
> +		status = "disabled";
> +	};
> +
> +	gic: interrupt-controller@10300000 {
> +		compatible = "arm,pl390";
> +		#interrupt-cells = <3>;
> +		interrupt-controller;
> +		reg = <0x10301000 0x1000>, <0x10302000 0x1000>;
> +	};
> +
> +	clk_3m: clk_3m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <3000000>;
> +	};
> +
> +	crg: clock-reset-controller@12040000 {
> +		compatible = "hisilicon,hi3521a-crg";
> +		#clock-cells = <1>;
> +		#reset-cells = <2>;
> +		reg = <0x12040000 0x10000>;
> +	};

These memory mapped peripherals should be under a bus node.

> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		interrupt-parent = <&gic>;
> +		ranges;

It is preferred to have a value here and limit the range of the bus 
addresses.

> +
> +		dmac: dma@10060000 {

dma-controller@...

> +			compatible = "arm,pl080", "arm,primecell";
> +			reg = <0x10060000 0x1000>;
> +			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";

I wouldn't think enabling dma would be a per board decision.

> +		};
> +
> +		dual_timer0: timer@12000000 {
> +			compatible = "arm,sp804", "arm,primecell";
> +			interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x12000000 0x1000>;
> +			clocks = <&clk_3m>;
> +			clock-names = "apb_pclk";

IIRC, it is deprecated to have a single clock here. The h/w has 2 clock 
inputs.

Where's the ARM architected timer?

> +			status = "disabled";
> +		};
> +
> +		dual_timer1: timer@12010000 {
> +			compatible = "arm,sp804", "arm,primecell";
> +			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x12010000 0x1000>;
> +			clocks = <&clk_3m>;
> +			clock-name = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		dual_timer2: timer@12020000 {
> +			compatible = "arm,sp804", "arm,primecell";
> +			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x12020000 0x1000>;
> +			clocks = <&clk_3m>;
> +			clock-name = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		dual_timer3: timer@12030000 {
> +			compatible = "arm,sp804", "arm,primecell";
> +			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x12030000 0x1000>;
> +			clocks = <&clk_3m>;
> +			clock-name = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		wdt0: watchdog@12070000 {
> +			compatible = "arm,sp805", "arm,primecell";
> +			arm,primecell-periphid = <0x00141805>;
> +			reg = <0x12070000 0x1000>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk_3m>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		uart0: serial@12080000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x12080000 0x1000>;
> +			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&crg HI3521A_UART0_CLK>;
> +			clock-names = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		uart1: serial@12090000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x12090000 0x1000>;
> +			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&crg HI3521A_UART1_CLK>;
> +			clock-names = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		uart2: serial@120a0000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x120a0000 0x1000>;
> +			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&crg HI3521A_UART2_CLK>;
> +			clock-names = "apb_pclk";
> +			status = "disabled";
> +		};
> +
> +		gpio0: gpio@12150000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12150000 0x1000>;
> +			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio1: gpio@12160000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12160000 0x1000>;
> +			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio2: gpio@12170000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12170000 0x1000>;
> +			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio3: gpio@12180000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12180000 0x1000>;
> +			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio4: gpio@12190000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12190000 0x1000>;
> +			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio5: gpio@121a0000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x121a0000 0x1000>;
> +			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio6: gpio@121b0000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x121b0000 0x1000>;
> +			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio7: gpio@121c0000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x121c0000 0x1000>;
> +			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio8: gpio@121d0000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x121d0000 0x1000>;
> +			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio9: gpio@121e0000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x121e0000 0x1000>;
> +			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio10: gpio@121f0000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x121f0000 0x1000>;
> +			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio11: gpio@12200000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12200000 0x1000>;
> +			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio12: gpio@12210000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12210000 0x1000>;
> +			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio13: gpio@12220000 {
> +			compatible = "arm,pl061", "arm,primecell";
> +			reg = <0x12220000 0x1000>;
> +			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			status = "disabled";
> +		};
> +	};
> +};
> -- 
> 2.14.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC RESEND 3/3] arm: dts: add Hi3521A dts
  2017-09-20 20:53     ` Rob Herring
@ 2017-09-20 23:04       ` Marty E. Plummer
       [not found]         ` <20170920230405.tz3vzoys2vn72rgv-1hXBbFsFebEOoWXT+9nehYcPyU6MP8rFhTcysPvq43Q@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Marty E. Plummer @ 2017-09-20 23:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, devicetree, xuejiancheng, leo.yan, linux-clk,
	linux-kernel, mark.rutland, mturquette, wenpan, linux, sboyd,
	xuwei5, zhangfei.gao, gregkh, arnd

On Wed, Sep 20, 2017 at 08:53:03PM +0000, Rob Herring wrote:
> On Sun, Sep 17, 2017 at 03:23:27AM -0500, Marty E. Plummer wrote:
> > Add hi3521a.dtsi and hi3521a-rs-dm290e.dts for RaySharp CCTV systems,
> > marketed under the name Samsung SDR-B74301N
> > 
> > Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
> > ---
> >  arch/arm/boot/dts/Makefile              |   2 +
> >  arch/arm/boot/dts/hi3521a-rs-dm290e.dts |  52 ++++++
> >  arch/arm/boot/dts/hi3521a.dtsi          | 310 ++++++++++++++++++++++++++++++++
> >  3 files changed, 364 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >  create mode 100644 arch/arm/boot/dts/hi3521a.dtsi
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index faf46abaa4a2..e7b9b5dde20f 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -189,6 +189,8 @@ dtb-$(CONFIG_ARCH_GEMINI) += \
> >  	gemini-sq201.dtb \
> >  	gemini-wbd111.dtb \
> >  	gemini-wbd222.dtb
> > +dtb-$(CONFIG_ARCH_HI3521A) += \
> > +	hi3521a-rs-dm290e.dtb
> >  dtb-$(CONFIG_ARCH_HI3xxx) += \
> >  	hi3620-hi4511.dtb
> >  dtb-$(CONFIG_ARCH_HIGHBANK) += \
> > diff --git a/arch/arm/boot/dts/hi3521a-rs-dm290e.dts b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> > new file mode 100644
> > index 000000000000..b32c8392c93f
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> > @@ -0,0 +1,52 @@
> > +/*
> > + * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> 
> Should be version 2 or later? Doesn't really matter to me from a DT 
> perspective, but it is in the kernel tree.
> 
> You can use SPDX tags if you want.
>
Oh, that's a good idea. I hadn't seen any SPDX tags in the tree that I
noticed before. I ended up just using the :Gpl command from neovim. 
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +/dts-v1/;
> > +#include "hi3521a.dtsi"
> > +
> > +/ {
> > +	model = "RaySharp RS-DM-290E DVR Board";
> > +	compatible = "hisilicon,hi3521a";
> 
> Needs a board compatible too.
>
Something like `compatible = "hisilicon,hi3521a", "raysharp,rs-dm-290e";` ? 
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +		serial1 = &uart1;
> > +		serial2 = &uart2;
> > +	};
> > +
> > +	memory {
> 
> Needs a unit-address.
>
Could you explain what you mean here? As in, memory@someaddr? What would
I use here? 
> > +		device_type = "memory";
> > +		reg = <0x80000000 0xf00000>;
> > +	};
> > +};
> > +
> > +&hi_sfc {
> > +	status = "okay";
> > +	spi-nor@0 {
> > +		compatible = "jedec,spi-nor";
> 
> I don't remember offhand, but I think this should have a device specific 
> compatible too.
>
Instead of "jedec,spi-nor" ? Specific to the SPI chip? 
> > +		reg = <0>;
> > +		spi-max-frequency = <104000000>;
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	status = "okay";
> > +};
> > +
> > +&dual_timer0 {
> > +	status = "okay";
> > +};
> > diff --git a/arch/arm/boot/dts/hi3521a.dtsi b/arch/arm/boot/dts/hi3521a.dtsi
> > new file mode 100644
> > index 000000000000..2af746fdec46
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/hi3521a.dtsi
> > @@ -0,0 +1,310 @@
> > +/*
> > + * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <dt-bindings/clock/hi3521a-clock.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +/ {
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	chosen { };
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu0: cpu@0 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a7";
> > +			reg = <0>;
> > +		};
> > +	};
> > +
> > +	hi_sfc: spi-nor-controller@10000000 {
> > +		compatible = "hisilicon,hi3521a-spi-nor", "hisilicon,fmc-spi-nor";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		reg = <0x10000000 0x10000>, <0x14000000 0x1000000>;
> > +		reg-names = "control", "memory";
> > +		clocks = <&crg HI3521A_FMC_CLK>;
> > +		status = "disabled";
> > +	};
> > +
> > +	gic: interrupt-controller@10300000 {
> > +		compatible = "arm,pl390";
> > +		#interrupt-cells = <3>;
> > +		interrupt-controller;
> > +		reg = <0x10301000 0x1000>, <0x10302000 0x1000>;
> > +	};
> > +
> > +	clk_3m: clk_3m {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <3000000>;
> > +	};
> > +
> > +	crg: clock-reset-controller@12040000 {
> > +		compatible = "hisilicon,hi3521a-crg";
> > +		#clock-cells = <1>;
> > +		#reset-cells = <2>;
> > +		reg = <0x12040000 0x10000>;
> > +	};
> 
> These memory mapped peripherals should be under a bus node.
>
Crap, will fix. 
> > +
> > +	soc {
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		compatible = "simple-bus";
> > +		interrupt-parent = <&gic>;
> > +		ranges;
> 
> It is preferred to have a value here and limit the range of the bus 
> addresses.
> 
Yeah, I think I've seen that before, I don't quite grok how that works.
> > +
> > +		dmac: dma@10060000 {
> 
> dma-controller@...
> 
Will fix.
> > +			compatible = "arm,pl080", "arm,primecell";
> > +			reg = <0x10060000 0x1000>;
> > +			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> > +			status = "disabled";
> 
> I wouldn't think enabling dma would be a per board decision.
> 
I've just noticed that in general dtsi files just lay it all out and are
mostly "disabled", though if you think this should be explicitly enabled
thats fine by me.
> > +		};
> > +
> > +		dual_timer0: timer@12000000 {
> > +			compatible = "arm,sp804", "arm,primecell";
> > +			interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > +			reg = <0x12000000 0x1000>;
> > +			clocks = <&clk_3m>;
> > +			clock-names = "apb_pclk";
> 
> IIRC, it is deprecated to have a single clock here. The h/w has 2 clock 
> inputs.
>
Are you meaning for the 0x0 index and 0x20 index clocks?
> Where's the ARM architected timer?
> 
Unsure tbqf, just doing my best to translate a datasheet into code. Do
all ARM soc's have one?
> > -- 
> > 2.14.1
> > 

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

* Re: [RFC RESEND 3/3] arm: dts: add Hi3521A dts
       [not found]         ` <20170920230405.tz3vzoys2vn72rgv-1hXBbFsFebEOoWXT+9nehYcPyU6MP8rFhTcysPvq43Q@public.gmane.org>
@ 2017-09-21  1:08           ` Rob Herring
  2017-09-21  2:15             ` Marty E. Plummer
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2017-09-21  1:08 UTC (permalink / raw)
  To: Marty E. Plummer
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jiancheng Xue,
	Leo Yan, linux-clk,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Mark Rutland, Michael Turquette, Pan Wen, Russell King,
	Stephen Boyd, Wei Xu, Zhangfei Gao, Greg Kroah-Hartman,
	Arnd Bergmann

On Wed, Sep 20, 2017 at 6:04 PM, Marty E. Plummer
<hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org> wrote:
> On Wed, Sep 20, 2017 at 08:53:03PM +0000, Rob Herring wrote:
>> On Sun, Sep 17, 2017 at 03:23:27AM -0500, Marty E. Plummer wrote:
>> > Add hi3521a.dtsi and hi3521a-rs-dm290e.dts for RaySharp CCTV systems,
>> > marketed under the name Samsung SDR-B74301N
>> >
>> > Signed-off-by: Marty E. Plummer <hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
>> > ---
>> >  arch/arm/boot/dts/Makefile              |   2 +
>> >  arch/arm/boot/dts/hi3521a-rs-dm290e.dts |  52 ++++++
>> >  arch/arm/boot/dts/hi3521a.dtsi          | 310 ++++++++++++++++++++++++++++++++
>> >  3 files changed, 364 insertions(+)
>> >  create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
>> >  create mode 100644 arch/arm/boot/dts/hi3521a.dtsi
>> >
>> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> > index faf46abaa4a2..e7b9b5dde20f 100644
>> > --- a/arch/arm/boot/dts/Makefile
>> > +++ b/arch/arm/boot/dts/Makefile
>> > @@ -189,6 +189,8 @@ dtb-$(CONFIG_ARCH_GEMINI) += \
>> >     gemini-sq201.dtb \
>> >     gemini-wbd111.dtb \
>> >     gemini-wbd222.dtb
>> > +dtb-$(CONFIG_ARCH_HI3521A) += \
>> > +   hi3521a-rs-dm290e.dtb
>> >  dtb-$(CONFIG_ARCH_HI3xxx) += \
>> >     hi3620-hi4511.dtb
>> >  dtb-$(CONFIG_ARCH_HIGHBANK) += \
>> > diff --git a/arch/arm/boot/dts/hi3521a-rs-dm290e.dts b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
>> > new file mode 100644
>> > index 000000000000..b32c8392c93f
>> > --- /dev/null
>> > +++ b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
>> > @@ -0,0 +1,52 @@
>> > +/*
>> > + * Copyright (C) 2017 Marty Plummer <hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
>> > + *
>> > + * This program is free software: you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation, either version 3 of the License, or
>> > + * (at your option) any later version.
>>
>> Should be version 2 or later? Doesn't really matter to me from a DT
>> perspective, but it is in the kernel tree.
>>
>> You can use SPDX tags if you want.
>>
> Oh, that's a good idea. I hadn't seen any SPDX tags in the tree that I
> noticed before. I ended up just using the :Gpl command from neovim.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> > + */
>> > +
>> > +/dts-v1/;
>> > +#include "hi3521a.dtsi"
>> > +
>> > +/ {
>> > +   model = "RaySharp RS-DM-290E DVR Board";
>> > +   compatible = "hisilicon,hi3521a";
>>
>> Needs a board compatible too.
>>
> Something like `compatible = "hisilicon,hi3521a", "raysharp,rs-dm-290e";` ?

Yes, but flip the order. Most specific compatible first.

>> > +
>> > +   aliases {
>> > +           serial0 = &uart0;
>> > +           serial1 = &uart1;
>> > +           serial2 = &uart2;
>> > +   };
>> > +
>> > +   memory {
>>
>> Needs a unit-address.
>>
> Could you explain what you mean here? As in, memory@someaddr? What would
> I use here?

"memory@80000000". Building with W=2 will tell you.

>> > +           device_type = "memory";
>> > +           reg = <0x80000000 0xf00000>;
>> > +   };
>> > +};
>> > +
>> > +&hi_sfc {
>> > +   status = "okay";
>> > +   spi-nor@0 {
>> > +           compatible = "jedec,spi-nor";
>>
>> I don't remember offhand, but I think this should have a device specific
>> compatible too.
>>
> Instead of "jedec,spi-nor" ? Specific to the SPI chip?

No, both with jedec,spi-nor 2nd.

>> > +           reg = <0>;
>> > +           spi-max-frequency = <104000000>;
>> > +   };
>> > +};
>> > +
>> > +&uart0 {
>> > +   status = "okay";
>> > +};
>> > +
>> > +&dual_timer0 {
>> > +   status = "okay";
>> > +};
>> > diff --git a/arch/arm/boot/dts/hi3521a.dtsi b/arch/arm/boot/dts/hi3521a.dtsi
>> > new file mode 100644
>> > index 000000000000..2af746fdec46
>> > --- /dev/null
>> > +++ b/arch/arm/boot/dts/hi3521a.dtsi
>> > @@ -0,0 +1,310 @@
>> > +/*
>> > + * Copyright (C) 2017 Marty Plummer <hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
>> > + *
>> > + * This program is free software: you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation, either version 3 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> > + */
>> > +
>> > +#include <dt-bindings/clock/hi3521a-clock.h>
>> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> > +/ {
>> > +   #address-cells = <1>;
>> > +   #size-cells = <1>;
>> > +   chosen { };
>> > +
>> > +   cpus {
>> > +           #address-cells = <1>;
>> > +           #size-cells = <0>;
>> > +
>> > +           cpu0: cpu@0 {
>> > +                   device_type = "cpu";
>> > +                   compatible = "arm,cortex-a7";
>> > +                   reg = <0>;
>> > +           };
>> > +   };
>> > +
>> > +   hi_sfc: spi-nor-controller@10000000 {
>> > +           compatible = "hisilicon,hi3521a-spi-nor", "hisilicon,fmc-spi-nor";
>> > +           #address-cells = <1>;
>> > +           #size-cells = <0>;
>> > +           reg = <0x10000000 0x10000>, <0x14000000 0x1000000>;
>> > +           reg-names = "control", "memory";
>> > +           clocks = <&crg HI3521A_FMC_CLK>;
>> > +           status = "disabled";
>> > +   };
>> > +
>> > +   gic: interrupt-controller@10300000 {
>> > +           compatible = "arm,pl390";
>> > +           #interrupt-cells = <3>;
>> > +           interrupt-controller;
>> > +           reg = <0x10301000 0x1000>, <0x10302000 0x1000>;
>> > +   };
>> > +
>> > +   clk_3m: clk_3m {
>> > +           compatible = "fixed-clock";
>> > +           #clock-cells = <0>;
>> > +           clock-frequency = <3000000>;
>> > +   };
>> > +
>> > +   crg: clock-reset-controller@12040000 {
>> > +           compatible = "hisilicon,hi3521a-crg";
>> > +           #clock-cells = <1>;
>> > +           #reset-cells = <2>;
>> > +           reg = <0x12040000 0x10000>;
>> > +   };
>>
>> These memory mapped peripherals should be under a bus node.
>>
> Crap, will fix.
>> > +
>> > +   soc {
>> > +           #address-cells = <1>;
>> > +           #size-cells = <1>;
>> > +           compatible = "simple-bus";
>> > +           interrupt-parent = <&gic>;
>> > +           ranges;
>>
>> It is preferred to have a value here and limit the range of the bus
>> addresses.
>>
> Yeah, I think I've seen that before, I don't quite grok how that works.
>> > +
>> > +           dmac: dma@10060000 {
>>
>> dma-controller@...
>>
> Will fix.
>> > +                   compatible = "arm,pl080", "arm,primecell";
>> > +                   reg = <0x10060000 0x1000>;
>> > +                   interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
>> > +                   status = "disabled";
>>
>> I wouldn't think enabling dma would be a per board decision.
>>
> I've just noticed that in general dtsi files just lay it all out and are
> mostly "disabled", though if you think this should be explicitly enabled
> thats fine by me.
>> > +           };
>> > +
>> > +           dual_timer0: timer@12000000 {
>> > +                   compatible = "arm,sp804", "arm,primecell";
>> > +                   interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
>> > +                                <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
>> > +                   reg = <0x12000000 0x1000>;
>> > +                   clocks = <&clk_3m>;
>> > +                   clock-names = "apb_pclk";
>>
>> IIRC, it is deprecated to have a single clock here. The h/w has 2 clock
>> inputs.
>>
> Are you meaning for the 0x0 index and 0x20 index clocks?

Yes. Maybe it's 3 clocks. Anyway, should all be in the sp804 binding doc.

>> Where's the ARM architected timer?
>>
> Unsure tbqf, just doing my best to translate a datasheet into code. Do
> all ARM soc's have one?

All A7's should I think.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC RESEND 3/3] arm: dts: add Hi3521A dts
  2017-09-21  1:08           ` Rob Herring
@ 2017-09-21  2:15             ` Marty E. Plummer
  0 siblings, 0 replies; 9+ messages in thread
From: Marty E. Plummer @ 2017-09-21  2:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, devicetree, xuejiancheng, leo.yan, linux-clk,
	linux-kernel@vger.kernel.org, mark.rutland, mturquette, wenpan,
	linux, sboyd, xuwei5, zhangfei.gao, gregkh, arnd

On Thu, Sep 21, 2017 at 01:08:39AM +0000, Rob Herring wrote:
> On Wed, Sep 20, 2017 at 6:04 PM, Marty E. Plummer
> <hanetzer@startmail.com> wrote:
> > On Wed, Sep 20, 2017 at 08:53:03PM +0000, Rob Herring wrote:
> >> On Sun, Sep 17, 2017 at 03:23:27AM -0500, Marty E. Plummer wrote:
> >> > Add hi3521a.dtsi and hi3521a-rs-dm290e.dts for RaySharp CCTV systems,
> >> > marketed under the name Samsung SDR-B74301N
> >> >
> >> > Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
> >> > ---
> >> >  arch/arm/boot/dts/Makefile              |   2 +
> >> >  arch/arm/boot/dts/hi3521a-rs-dm290e.dts |  52 ++++++
> >> >  arch/arm/boot/dts/hi3521a.dtsi          | 310 ++++++++++++++++++++++++++++++++
> >> >  3 files changed, 364 insertions(+)
> >> >  create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >> >  create mode 100644 arch/arm/boot/dts/hi3521a.dtsi
> >> >
> >> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >> > index faf46abaa4a2..e7b9b5dde20f 100644
> >> > --- a/arch/arm/boot/dts/Makefile
> >> > +++ b/arch/arm/boot/dts/Makefile
> >> > @@ -189,6 +189,8 @@ dtb-$(CONFIG_ARCH_GEMINI) += \
> >> >     gemini-sq201.dtb \
> >> >     gemini-wbd111.dtb \
> >> >     gemini-wbd222.dtb
> >> > +dtb-$(CONFIG_ARCH_HI3521A) += \
> >> > +   hi3521a-rs-dm290e.dtb
> >> >  dtb-$(CONFIG_ARCH_HI3xxx) += \
> >> >     hi3620-hi4511.dtb
> >> >  dtb-$(CONFIG_ARCH_HIGHBANK) += \
> >> > diff --git a/arch/arm/boot/dts/hi3521a-rs-dm290e.dts b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >> > new file mode 100644
> >> > index 000000000000..b32c8392c93f
> >> > --- /dev/null
> >> > +++ b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >> > @@ -0,0 +1,52 @@
> >> > +/*
> >> > + * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
> >> > + *
> >> > + * This program is free software: you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation, either version 3 of the License, or
> >> > + * (at your option) any later version.
> >>
> >> Should be version 2 or later? Doesn't really matter to me from a DT
> >> perspective, but it is in the kernel tree.
> >>
> >> You can use SPDX tags if you want.
> >>
> > Oh, that's a good idea. I hadn't seen any SPDX tags in the tree that I
> > noticed before. I ended up just using the :Gpl command from neovim.
> >> > + *
> >> > + * This program is distributed in the hope that it will be useful,
> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> > + * GNU General Public License for more details.
> >> > + *
> >> > + * You should have received a copy of the GNU General Public License
> >> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >> > + */
> >> > +
> >> > +/dts-v1/;
> >> > +#include "hi3521a.dtsi"
> >> > +
> >> > +/ {
> >> > +   model = "RaySharp RS-DM-290E DVR Board";
> >> > +   compatible = "hisilicon,hi3521a";
> >>
> >> Needs a board compatible too.
> >>
> > Something like `compatible = "hisilicon,hi3521a", "raysharp,rs-dm-290e";` ?
> 
> Yes, but flip the order. Most specific compatible first.
> 
> >> > +
> >> > +   aliases {
> >> > +           serial0 = &uart0;
> >> > +           serial1 = &uart1;
> >> > +           serial2 = &uart2;
> >> > +   };
> >> > +
> >> > +   memory {
> >>
> >> Needs a unit-address.
> >>
> > Could you explain what you mean here? As in, memory@someaddr? What would
> > I use here?
> 
> "memory@80000000". Building with W=2 will tell you.
> 
Ah, nice trick. Suppose that makes sense, as every other thing was the
same on that sort of thing. Not sure if I've ever seen memory@addr
before.
> >> > +           device_type = "memory";
> >> > +           reg = <0x80000000 0xf00000>;
> >> > +   };
> >> > +};
> >> > +
> >> > +&hi_sfc {
> >> > +   status = "okay";
> >> > +   spi-nor@0 {
> >> > +           compatible = "jedec,spi-nor";
> >>
> >> I don't remember offhand, but I think this should have a device specific
> >> compatible too.
> >>
> > Instead of "jedec,spi-nor" ? Specific to the SPI chip?
> 
> No, both with jedec,spi-nor 2nd.
> 
Gotcha, will fix it up.
> >> > +           reg = <0>;
> >> > +           spi-max-frequency = <104000000>;
> >> > +   };
> >> > +};
> >> > +
> >> > +&uart0 {
> >> > +   status = "okay";
> >> > +};
> >> > +
> >> > +&dual_timer0 {
> >> > +   status = "okay";
> >> > +};
> >> > diff --git a/arch/arm/boot/dts/hi3521a.dtsi b/arch/arm/boot/dts/hi3521a.dtsi
> >> > new file mode 100644
> >> > index 000000000000..2af746fdec46
> >> > --- /dev/null
> >> > +++ b/arch/arm/boot/dts/hi3521a.dtsi
> >> > @@ -0,0 +1,310 @@
> >> > +/*
> >> > + * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
> >> > + *
> >> > + * This program is free software: you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation, either version 3 of the License, or
> >> > + * (at your option) any later version.
> >> > + *
> >> > + * This program is distributed in the hope that it will be useful,
> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> > + * GNU General Public License for more details.
> >> > + *
> >> > + * You should have received a copy of the GNU General Public License
> >> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >> > + */
> >> > +
> >> > +#include <dt-bindings/clock/hi3521a-clock.h>
> >> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> >> > +/ {
> >> > +   #address-cells = <1>;
> >> > +   #size-cells = <1>;
> >> > +   chosen { };
> >> > +
> >> > +   cpus {
> >> > +           #address-cells = <1>;
> >> > +           #size-cells = <0>;
> >> > +
> >> > +           cpu0: cpu@0 {
> >> > +                   device_type = "cpu";
> >> > +                   compatible = "arm,cortex-a7";
> >> > +                   reg = <0>;
> >> > +           };
> >> > +   };
> >> > +
> >> > +   hi_sfc: spi-nor-controller@10000000 {
> >> > +           compatible = "hisilicon,hi3521a-spi-nor", "hisilicon,fmc-spi-nor";
> >> > +           #address-cells = <1>;
> >> > +           #size-cells = <0>;
> >> > +           reg = <0x10000000 0x10000>, <0x14000000 0x1000000>;
> >> > +           reg-names = "control", "memory";
> >> > +           clocks = <&crg HI3521A_FMC_CLK>;
> >> > +           status = "disabled";
> >> > +   };
> >> > +
> >> > +   gic: interrupt-controller@10300000 {
> >> > +           compatible = "arm,pl390";
> >> > +           #interrupt-cells = <3>;
> >> > +           interrupt-controller;
> >> > +           reg = <0x10301000 0x1000>, <0x10302000 0x1000>;
> >> > +   };
> >> > +
> >> > +   clk_3m: clk_3m {
> >> > +           compatible = "fixed-clock";
> >> > +           #clock-cells = <0>;
> >> > +           clock-frequency = <3000000>;
> >> > +   };
> >> > +
> >> > +   crg: clock-reset-controller@12040000 {
> >> > +           compatible = "hisilicon,hi3521a-crg";
> >> > +           #clock-cells = <1>;
> >> > +           #reset-cells = <2>;
> >> > +           reg = <0x12040000 0x10000>;
> >> > +   };
> >>
> >> These memory mapped peripherals should be under a bus node.
> >>
> > Crap, will fix.
> >> > +
> >> > +   soc {
> >> > +           #address-cells = <1>;
> >> > +           #size-cells = <1>;
> >> > +           compatible = "simple-bus";
> >> > +           interrupt-parent = <&gic>;
> >> > +           ranges;
> >>
> >> It is preferred to have a value here and limit the range of the bus
> >> addresses.
> >>
> > Yeah, I think I've seen that before, I don't quite grok how that works.
> >> > +
> >> > +           dmac: dma@10060000 {
> >>
> >> dma-controller@...
> >>
> > Will fix.
> >> > +                   compatible = "arm,pl080", "arm,primecell";
> >> > +                   reg = <0x10060000 0x1000>;
> >> > +                   interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> >> > +                   status = "disabled";
> >>
> >> I wouldn't think enabling dma would be a per board decision.
> >>
> > I've just noticed that in general dtsi files just lay it all out and are
> > mostly "disabled", though if you think this should be explicitly enabled
> > thats fine by me.
> >> > +           };
> >> > +
> >> > +           dual_timer0: timer@12000000 {
> >> > +                   compatible = "arm,sp804", "arm,primecell";
> >> > +                   interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> >> > +                                <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> >> > +                   reg = <0x12000000 0x1000>;
> >> > +                   clocks = <&clk_3m>;
> >> > +                   clock-names = "apb_pclk";
> >>
> >> IIRC, it is deprecated to have a single clock here. The h/w has 2 clock
> >> inputs.
> >>
> > Are you meaning for the 0x0 index and 0x20 index clocks?
> 
> Yes. Maybe it's 3 clocks. Anyway, should all be in the sp804 binding doc.
> 
> >> Where's the ARM architected timer?
> >>
> > Unsure tbqf, just doing my best to translate a datasheet into code. Do
> > all ARM soc's have one?
> 
> All A7's should I think.
> 
Gotcha.
> Rob

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

end of thread, other threads:[~2017-09-21  2:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-17  8:23 [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Marty E. Plummer
2017-09-17  8:23 ` [RFC RESEND 1/3] clk: hisilicon: add CRG driver " Marty E. Plummer
2017-09-17  8:23 ` [RFC RESEND 2/3] arm: hisi: enable " Marty E. Plummer
2017-09-17  8:23 ` [RFC RESEND 3/3] arm: dts: add Hi3521A dts Marty E. Plummer
     [not found]   ` <20170917082327.10058-4-hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
2017-09-20 20:53     ` Rob Herring
2017-09-20 23:04       ` Marty E. Plummer
     [not found]         ` <20170920230405.tz3vzoys2vn72rgv-1hXBbFsFebEOoWXT+9nehYcPyU6MP8rFhTcysPvq43Q@public.gmane.org>
2017-09-21  1:08           ` Rob Herring
2017-09-21  2:15             ` Marty E. Plummer
2017-09-18 10:55 ` [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Greg KH

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