[PATCH 0/3] ARM: clps711x: Initial DT support

[PATCH 0/3] ARM: clps711x: Initial DT support

[Alexander Shiyan]

This small series of patches adds initial DT support for Cirrus Logic
ARMv4T processors.

Alexander Shiyan (3):
  ARM: clps711x: Reduce static map size
  ARM: clps711x: Add basic DT support
  ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

 arch/arm/Kconfig                               |   2 +
 arch/arm/boot/dts/Makefile                     |   2 +
 arch/arm/boot/dts/clps711x-cdb89712.dts        |  56 +++++++
 arch/arm/boot/dts/clps711x.dtsi                | 201 +++++++++++++++++++++++++
 arch/arm/include/debug/clps711x.S              |   4 +-
 arch/arm/mach-clps711x/Kconfig                 |   7 +
 arch/arm/mach-clps711x/common.c                |  20 ++-
 arch/arm/mach-clps711x/devices.c               |   6 +
 arch/arm/mach-clps711x/devices.h               |   1 +
 arch/arm/mach-clps711x/include/mach/hardware.h |   2 +-
 10 files changed, 295 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/boot/dts/clps711x-cdb89712.dts
 create mode 100644 arch/arm/boot/dts/clps711x.dtsi

-- 
2.4.9

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

[PATCH 1/3] ARM: clps711x: Reduce static map size

[Alexander Shiyan]

Last CLPS711X CPU register is PLLR has 0xa5a8 address, so we can reduce
the map to 48k and align the end of the static at VMALLOC_START.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/include/debug/clps711x.S              | 4 ++--
 arch/arm/mach-clps711x/common.c                | 6 +++---
 arch/arm/mach-clps711x/include/mach/hardware.h | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/debug/clps711x.S b/arch/arm/include/debug/clps711x.S
index abe2254..c17ac5c 100644
--- a/arch/arm/include/debug/clps711x.S
+++ b/arch/arm/include/debug/clps711x.S
@@ -9,10 +9,10 @@
 
 #ifndef CONFIG_DEBUG_CLPS711X_UART2
 #define CLPS711X_UART_PADDR	(0x80000000 + 0x0000)
-#define CLPS711X_UART_VADDR	(0xfeff0000 + 0x0000)
+#define CLPS711X_UART_VADDR	(0xfeff4000 + 0x0000)
 #else
 #define CLPS711X_UART_PADDR	(0x80000000 + 0x1000)
-#define CLPS711X_UART_VADDR	(0xfeff0000 + 0x1000)
+#define CLPS711X_UART_VADDR	(0xfeff4000 + 0x1000)
 #endif
 
 #define SYSFLG		(0x0140)
diff --git a/arch/arm/mach-clps711x/common.c b/arch/arm/mach-clps711x/common.c
index 671acc5a..bf62211 100644
--- a/arch/arm/mach-clps711x/common.c
+++ b/arch/arm/mach-clps711x/common.c
@@ -37,9 +37,9 @@ static struct map_desc clps711x_io_desc[] __initdata = {
 	{
 		.virtual	= (unsigned long)CLPS711X_VIRT_BASE,
 		.pfn		= __phys_to_pfn(CLPS711X_PHYS_BASE),
-		.length		= SZ_64K,
-		.type		= MT_DEVICE
-	}
+		.length		= 48 * SZ_1K,
+		.type		= MT_DEVICE,
+	},
 };
 
 void __init clps711x_map_io(void)
diff --git a/arch/arm/mach-clps711x/include/mach/hardware.h b/arch/arm/mach-clps711x/include/mach/hardware.h
index 833129c..cfd0e2c 100644
--- a/arch/arm/mach-clps711x/include/mach/hardware.h
+++ b/arch/arm/mach-clps711x/include/mach/hardware.h
@@ -24,7 +24,7 @@
 
 #include <mach/clps711x.h>
 
-#define CLPS711X_VIRT_BASE	IOMEM(0xfeff0000)
+#define CLPS711X_VIRT_BASE	IOMEM(0xfeff4000)
 
 #ifndef __ASSEMBLY__
 #define clps_readb(off)		readb(CLPS711X_VIRT_BASE + (off))
-- 
2.4.9

[PATCH 2/3] ARM: clps711x: Add basic DT support

[Alexander Shiyan]

This patch adds basic support to run Cirrus Logic ARMv4T CPUs
with device-tree support.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/Kconfig                 |  2 ++
 arch/arm/mach-clps711x/Kconfig   |  7 +++++++
 arch/arm/mach-clps711x/common.c  | 14 ++++++++++++++
 arch/arm/mach-clps711x/devices.c |  6 ++++++
 arch/arm/mach-clps711x/devices.h |  1 +
 5 files changed, 30 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index cdfa6c2..afc1eaf 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -357,10 +357,12 @@ config ARCH_CLPS711X
 	select ARCH_REQUIRE_GPIOLIB
 	select AUTO_ZRELADDR
 	select CLKSRC_MMIO
+	select CLKSRC_OF if OF
 	select COMMON_CLK
 	select CPU_ARM720T
 	select GENERIC_CLOCKEVENTS
 	select MFD_SYSCON
+	select OF_IRQ if OF
 	select SOC_BUS
 	help
 	  Support for Cirrus Logic 711x/721x/731x based boards.
diff --git a/arch/arm/mach-clps711x/Kconfig b/arch/arm/mach-clps711x/Kconfig
index f711498..3e11390 100644
--- a/arch/arm/mach-clps711x/Kconfig
+++ b/arch/arm/mach-clps711x/Kconfig
@@ -2,6 +2,13 @@ if ARCH_CLPS711X
 
 menu "CLPS711X/EP721X/EP731X Implementations"
 
+config MACH_CLPS711X_DT
+	bool "Device-tree support"
+	select USE_OF
+	help
+	  Select this if you want to experiment device-tree with
+	  ARMv4T Cirrus Logic chips.
+
 config ARCH_AUTCPU12
 	bool "AUTCPU12"
 	help
diff --git a/arch/arm/mach-clps711x/common.c b/arch/arm/mach-clps711x/common.c
index bf62211..df0fdf8 100644
--- a/arch/arm/mach-clps711x/common.c
+++ b/arch/arm/mach-clps711x/common.c
@@ -23,12 +23,14 @@
 #include <linux/init.h>
 #include <linux/sizes.h>
 
+#include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/system_misc.h>
 
 #include <mach/hardware.h>
 
 #include "common.h"
+#include "devices.h"
 
 /*
  * This maps the generic CLPS711x registers
@@ -63,3 +65,15 @@ void clps711x_restart(enum reboot_mode mode, const char *cmd)
 {
 	soft_restart(0);
 }
+
+static const char *clps711x_dt_compat[] __initconst = {
+	"cirrus,clps711x",
+	NULL
+};
+
+DT_MACHINE_START(CLPS711X_DT, "Cirrus Logic CLPS711X (Device Tree Support)")
+	.dt_compat	= clps711x_dt_compat,
+	.map_io		= clps711x_map_io,
+	.init_late	= clps711x_devices_init_dt,
+	.restart	= clps711x_restart,
+MACHINE_END
diff --git a/arch/arm/mach-clps711x/devices.c b/arch/arm/mach-clps711x/devices.c
index 77a9617..9ba5095 100644
--- a/arch/arm/mach-clps711x/devices.c
+++ b/arch/arm/mach-clps711x/devices.c
@@ -147,3 +147,9 @@ void __init clps711x_devices_init(void)
 	clps711x_add_uart();
 	clps711x_soc_init();
 }
+
+void __init clps711x_devices_init_dt(void)
+{
+	clps711x_add_cpuidle();
+	clps711x_soc_init();
+};
diff --git a/arch/arm/mach-clps711x/devices.h b/arch/arm/mach-clps711x/devices.h
index a5efc17..47378cd 100644
--- a/arch/arm/mach-clps711x/devices.h
+++ b/arch/arm/mach-clps711x/devices.h
@@ -10,3 +10,4 @@
  */
 
 void clps711x_devices_init(void);
+void clps711x_devices_init_dt(void);
-- 
2.4.9

[PATCH 3/3] ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

[Alexander Shiyan]

This adds the Cirrus Logic CLPS711X DT template and support for
CDB89712 Development board.
http://www.cirrus.com/en/pubs/manual/cdb89712-2.pdf

Signed-off-by: Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
---
 arch/arm/boot/dts/Makefile              |   2 +
 arch/arm/boot/dts/clps711x-cdb89712.dts |  56 +++++++++
 arch/arm/boot/dts/clps711x.dtsi         | 201 ++++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+)
 create mode 100644 arch/arm/boot/dts/clps711x-cdb89712.dts
 create mode 100644 arch/arm/boot/dts/clps711x.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 95c1923..69de1d5 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -104,6 +104,8 @@ dtb-$(CONFIG_ARCH_BERLIN) += \
 	berlin2q-marvell-dmp.dtb
 dtb-$(CONFIG_ARCH_BRCMSTB) += \
 	bcm7445-bcm97445svmb.dtb
+dtb-$(CONFIG_ARCH_CLPS711X) += \
+	clps711x-cdb89712.dtb
 dtb-$(CONFIG_ARCH_DAVINCI) += \
 	da850-enbw-cmc.dtb \
 	da850-evm.dtb
diff --git a/arch/arm/boot/dts/clps711x-cdb89712.dts b/arch/arm/boot/dts/clps711x-cdb89712.dts
new file mode 100644
index 0000000..eaffd7c
--- /dev/null
+++ b/arch/arm/boot/dts/clps711x-cdb89712.dts
@@ -0,0 +1,56 @@
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ */
+
+#include "clps711x.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	model = "Cirrus Logic CS89712 Development Board";
+	compatible = "cirrus,cdb89712", "cirrus,cs89712", "cirrus,clps711x";
+
+	memory {
+		reg = <0xc0000000 0x01000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		pd0 {
+			label = "diagnostic";
+			gpios = <&portd 0 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "heartbeat";
+		};
+	};
+};
+
+&bus {
+	flash: root@00000000 {
+		compatible = "cfi-flash";
+		reg = <0 0x00000000 0x01000000>;
+		bank-width = <2>;
+		linux,mtd-name = "physmap-flash.0";
+		#address-cells = <1>;
+		#size-cells = <1>;
+	};
+
+	boot: boot@0x70000000 {
+		compatible = "cfi-flash";
+		reg = <7 0x00000000 0x00000080>;
+		bank-width = <2>;
+		linux,mtd-name = "physmap-flash.1";
+		#address-cells = <1>;
+		#size-cells = <1>;
+	};
+};
+
+&uart1 {
+	cts-gpios = <&mctrl 0 GPIO_ACTIVE_LOW>;
+	dsr-gpios = <&mctrl 1 GPIO_ACTIVE_LOW>;
+	dcd-gpios = <&mctrl 2 GPIO_ACTIVE_LOW>;
+	rts-gpios = <&portb 1 GPIO_ACTIVE_LOW>;
+	rng-gpios = <&portb 2 GPIO_ACTIVE_LOW>;
+};
diff --git a/arch/arm/boot/dts/clps711x.dtsi b/arch/arm/boot/dts/clps711x.dtsi
new file mode 100644
index 0000000..791eeee
--- /dev/null
+++ b/arch/arm/boot/dts/clps711x.dtsi
@@ -0,0 +1,201 @@
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ */
+
+/dts-v1/;
+
+#include "skeleton.dtsi"
+
+#include <dt-bindings/clock/clps711x-clock.h>
+
+/ {
+	model = "Cirrus Logic CLPS711X";
+	compatible = "cirrus,clps711x";
+
+	aliases {
+		gpio0 = &porta;
+		gpio1 = &portb;
+		gpio2 = &portc;
+		gpio3 = &portd;
+		gpio4 = &porte;
+		serial0 = &uart1;
+		serial1 = &uart2;
+		spi0 = &spi;
+		timer0 = &timer1;
+		timer1 = &timer2;
+	};
+
+	cpus {
+		#address-cells = <0>;
+		#size-cells = <0>;
+
+		cpu {
+			device_type = "cpu";
+			compatible = "arm,arm720t";
+		};
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&intc>;
+		ranges;
+
+		clks: clks@80000000 {
+			#clock-cells = <1>;
+			compatible = "cirrus,clps711x-clk";
+			reg = <0x80000000 0xc000>;
+			startup-frequency = <73728000>;
+		};
+
+		intc: intc@80000000 {
+			compatible = "cirrus,clps711x-intc";
+			reg = <0x80000000 0x4000>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+
+		porta: gpio@80000000 {
+			compatible = "cirrus,clps711x-gpio";
+			reg = <0x80000000 0x1 0x80000040 0x1>;
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		portb: gpio@80000001 {
+			compatible = "cirrus,clps711x-gpio";
+			reg = <0x80000001 0x1 0x80000041 0x1>;
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		portc: gpio@80000002 {
+			compatible = "cirrus,clps711x-gpio";
+			reg = <0x80000002 0x1 0x80000042 0x1>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			status = "disabled";
+		};
+
+		portd: gpio@80000003 {
+			compatible = "cirrus,clps711x-gpio";
+			reg = <0x80000003 0x1 0x80000043 0x1>;
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		porte: gpio@80000083 {
+			compatible = "cirrus,clps711x-gpio";
+			reg = <0x80000083 0x1 0x800000c3 0x1>;
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		syscon1: syscon@80000100 {
+			compatible = "cirrus,clps711x-syscon1", "syscon";
+			reg = <0x80000100 0x80>;
+		};
+
+		bus: bus@80000180 {
+			#address-cells = <2>;
+			#size-cells = <1>;
+			compatible = "cirrus,clps711x-bus", "simple-bus";
+			clocks = <&clks CLPS711X_CLK_BUS>;
+			reg = <0x80000180 0x80>;
+			ranges = <
+				0 0 0x00000000 0x10000000
+				1 0 0x10000000 0x10000000
+				2 0 0x20000000 0x10000000
+				3 0 0x30000000 0x10000000
+				4 0 0x40000000 0x10000000
+				5 0 0x50000000 0x10000000
+				6 0 0x60000000 0x0000c000
+				7 0 0x70000000 0x00000080
+			>;
+		};
+
+		fb: fb@800002c0 {
+			compatible = "cirrus,clps711x-fb";
+			reg = <0x800002c0 0xd44>, <0x60000000 0xc000>;
+			clocks = <&clks CLPS711X_CLK_BUS>;
+			status = "disabled";
+		};
+
+		timer1: timer@80000300 {
+			compatible = "cirrus,clps711x-timer";
+			reg = <0x80000300 0x4>;
+			clocks = <&clks CLPS711X_CLK_TIMER1>;
+			interrupts = <8>;
+		};
+
+		timer2: timer@80000340 {
+			compatible = "cirrus,clps711x-timer";
+			reg = <0x80000340 0x4>;
+			clocks = <&clks CLPS711X_CLK_TIMER2>;
+			interrupts = <9>;
+		};
+
+		pwm: pwm@80000400 {
+			compatible = "cirrus,clps711x-pwm";
+			reg = <0x80000400 0x4>;
+			clocks = <&clks CLPS711X_CLK_PWM>;
+			#pwm-cells = <1>;
+		};
+
+		uart1: uart@80000480 {
+			compatible = "cirrus,clps711x-uart";
+			reg = <0x80000480 0x80>;
+			interrupts = <12 13>;
+			clocks = <&clks CLPS711X_CLK_UART>;
+			syscon = <&syscon1>;
+		};
+
+		spi: spi@80000500 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "cirrus,clps711x-spi";
+			reg = <0x80000500 0x4>;
+			interrupts = <15>;
+			clocks = <&clks CLPS711X_CLK_SPI>;
+			status = "disabled";
+		};
+
+		syscon2: syscon@80001100 {
+			compatible = "cirrus,clps711x-syscon2", "syscon";
+			reg = <0x80001100 0x80>;
+		};
+
+		uart2: uart@80001480 {
+			compatible = "cirrus,clps711x-uart";
+			reg = <0x80001480 0x80>;
+			interrupts = <28 29>;
+			clocks = <&clks CLPS711X_CLK_UART>;
+			syscon = <&syscon2>;
+		};
+
+		dai: dai@80002000 {
+			#sound-dai-cells = <0>;
+			compatible = "cirrus,clps711x-dai";
+			reg = <0x80002000 0x604>;
+			clocks = <&clks CLPS711X_CLK_PLL>;
+			clock-names = "pll";
+			interrupts = <32>;
+			status = "disabled";
+		};
+
+		syscon3: syscon@80002200 {
+			compatible = "cirrus,clps711x-syscon3", "syscon";
+			reg = <0x80002200 0x40>;
+		};
+	};
+
+	mctrl: mctrl {
+		compatible = "cirrus,clps711x-mctrl-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+};
-- 
2.4.9

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

Re: [PATCH 3/3] ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

[Arnd Bergmann]

On Friday 13 May 2016 14:26:28 Alexander Shiyan wrote:
> +&bus {
> +	flash: root@00000000 {
> +		compatible = "cfi-flash";
> +		reg = <0 0x00000000 0x01000000>;
> +		bank-width = <2>;
> +		linux,mtd-name = "physmap-flash.0";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +	};

I've never seen the linux,mtd-name property before, but I think it's
meant to refer to the name of the flash chip, not the
name of the linux device.

> +	aliases {
> +		gpio0 = &porta;
> +		gpio1 = &portb;
> +		gpio2 = &portc;
> +		gpio3 = &portd;
> +		gpio4 = &porte;
> +		serial0 = &uart1;
> +		serial1 = &uart2;
> +		spi0 = &spi;
> +		timer0 = &timer1;
> +		timer1 = &timer2;
> +	};

Please move this into the .dts file: not all boards necessarily have
all of the above.

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

All child devices seem to be in the 0x80000000-0x90000000
range, maybe set the ranges property accordingly?

> +		clks: clks@80000000 {
> +			#clock-cells = <1>;
> +			compatible = "cirrus,clps711x-clk";
> +			reg = <0x80000000 0xc000>;
> +			startup-frequency = <73728000>;
> +		};

Please fix the compatible strings for all devices to not
have 'x' for wildcards. Normally you'd use the name of hte
first chip to have this particular IP block.

> +		intc: intc@80000000 {
> +			compatible = "cirrus,clps711x-intc";
> +			reg = <0x80000000 0x4000>;
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +		};

Better make the register ranges non-overlapping. This appears
to start at the same place as the 'clks' node.

> +		porta: gpio@80000000 {
> +			compatible = "cirrus,clps711x-gpio";
> +			reg = <0x80000000 0x1 0x80000040 0x1>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
> +
> +		portb: gpio@80000001 {
> +			compatible = "cirrus,clps711x-gpio";
> +			reg = <0x80000001 0x1 0x80000041 0x1>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
> +
> +		portc: gpio@80000002 {
> +			compatible = "cirrus,clps711x-gpio";
> +			reg = <0x80000002 0x1 0x80000042 0x1>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			status = "disabled";
> +		};
> +
> +		portd: gpio@80000003 {
> +			compatible = "cirrus,clps711x-gpio";
> +			reg = <0x80000003 0x1 0x80000043 0x1>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};

These are all just single bytes. My guess is that there is
actually one IP block that contains all the gpios, not
four identical blocks.

> +		bus: bus@80000180 {
> +			#address-cells = <2>;
> +			#size-cells = <1>;
> +			compatible = "cirrus,clps711x-bus", "simple-bus";
> +			clocks = <&clks CLPS711X_CLK_BUS>;
> +			reg = <0x80000180 0x80>;

If it has registers, it's not a 'simple-bus'. Is this an external
bus controller perhaps?

	Arnd
--
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

Re: [PATCH 3/3] ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

[Alexander Shiyan]

Hello.

> Пятница, 13 мая 2016, 15:00 +03:00 от Arnd Bergmann <arnd@arndb.de>:
> 
> On Friday 13 May 2016 14:26:28 Alexander Shiyan wrote:
...
> > +	soc {
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		compatible = "simple-bus";
> > +		interrupt-parent = <&intc>;
> > +		ranges;
> 
> All child devices seem to be in the 0x80000000-0x90000000
> range, maybe set the ranges property accordingly?

I do not quite understand that you want to change here.

> 
> > +		clks: clks@80000000 {
> > +			#clock-cells = <1>;
> > +			compatible = "cirrus,clps711x-clk";
> > +			reg = <0x80000000 0xc000>;
> > +			startup-frequency = <73728000>;
> > +		};
...
> > +		intc: intc@80000000 {
> > +			compatible = "cirrus,clps711x-intc";
> > +			reg = <0x80000000 0x4000>;
> > +			interrupt-controller;
> > +			#interrupt-cells = <1>;
> > +		};
> 
> Better make the register ranges non-overlapping. This appears
> to start at the same place as the 'clks' node.

CLK driver uses:
#define CLPS711X_SYSCON1 (0x0100)
#define CLPS711X_SYSCON2 (0x1100)
...
#define CLPS711X_PLLR (0xa5a8)

IRQCHIP driver uses:
#define CLPS711X_INTSR1 (0x0240)
...
#define CLPS711X_INTSR2 (0x1240)
...
#define CLPS711X_INTMR3 (0x2280)

So there is no way to do any else.

> > +		porta: gpio@80000000 {
> > +			compatible = "cirrus,clps711x-gpio";
> > +			reg = <0x80000000 0x1 0x80000040 0x1>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +
> > +		portb: gpio@80000001 {
> > +			compatible = "cirrus,clps711x-gpio";
> > +			reg = <0x80000001 0x1 0x80000041 0x1>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +
> > +		portc: gpio@80000002 {
> > +			compatible = "cirrus,clps711x-gpio";
> > +			reg = <0x80000002 0x1 0x80000042 0x1>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			status = "disabled";
> > +		};
> > +
> > +		portd: gpio@80000003 {
> > +			compatible = "cirrus,clps711x-gpio";
> > +			reg = <0x80000003 0x1 0x80000043 0x1>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> 
> These are all just single bytes. My guess is that there is
> actually one IP block that contains all the gpios, not
> four identical blocks.

These is a 8-bit registers with different properties, which parses in the
GPIO driver depending of alias (GPIO count, inverted logic etc.).

Thanks.

---

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

[Arnd Bergmann]

On Friday 13 May 2016 15:30:23 Alexander Shiyan wrote:
> Hello.
> 
> > Пятница, 13 мая 2016, 15:00 +03:00 от Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> > 
> > On Friday 13 May 2016 14:26:28 Alexander Shiyan wrote:
> ...
> > > +	soc {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <1>;
> > > +		compatible = "simple-bus";
> > > +		interrupt-parent = <&intc>;
> > > +		ranges;
> > 
> > All child devices seem to be in the 0x80000000-0x90000000
> > range, maybe set the ranges property accordingly?
> 
> I do not quite understand that you want to change here.

I meant using the ranges property to describe the bus addresses
without the 0x80000000 offset, like

	ranges = <0 0x8000000 0xc000>;

and then adapting the registers under the node to be based
on the bus address.


> > > +		clks: clks@80000000 {
> > > +			#clock-cells = <1>;
> > > +			compatible = "cirrus,clps711x-clk";
> > > +			reg = <0x80000000 0xc000>;
> > > +			startup-frequency = <73728000>;
> > > +		};
> ...
> > > +		intc: intc@80000000 {
> > > +			compatible = "cirrus,clps711x-intc";
> > > +			reg = <0x80000000 0x4000>;
> > > +			interrupt-controller;
> > > +			#interrupt-cells = <1>;
> > > +		};
> > 
> > Better make the register ranges non-overlapping. This appears
> > to start at the same place as the 'clks' node.
> 
> CLK driver uses:
> #define CLPS711X_SYSCON1 (0x0100)
> #define CLPS711X_SYSCON2 (0x1100)
> ...
> #define CLPS711X_PLLR (0xa5a8)
> 
> IRQCHIP driver uses:
> #define CLPS711X_INTSR1 (0x0240)
> ...
> #define CLPS711X_INTSR2 (0x1240)
> ...
> #define CLPS711X_INTMR3 (0x2280)
> 
> So there is no way to do any else.

It sounds like what you have is a large system controller that
has registers everywhere. Could you use syscon to access the
individual registers instead?

> > > +		porta: gpio@80000000 {
> > > +			compatible = "cirrus,clps711x-gpio";
> > > +			reg = <0x80000000 0x1 0x80000040 0x1>;
> > > +			gpio-controller;
> > > +			#gpio-cells = <2>;
> > > +		};
> > > +
> > > +		portb: gpio@80000001 {
> > > +			compatible = "cirrus,clps711x-gpio";
> > > +			reg = <0x80000001 0x1 0x80000041 0x1>;
> > > +			gpio-controller;
> > > +			#gpio-cells = <2>;
> > > +		};
> > > +
> > > +		portc: gpio@80000002 {
> > > +			compatible = "cirrus,clps711x-gpio";
> > > +			reg = <0x80000002 0x1 0x80000042 0x1>;
> > > +			gpio-controller;
> > > +			#gpio-cells = <2>;
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		portd: gpio@80000003 {
> > > +			compatible = "cirrus,clps711x-gpio";
> > > +			reg = <0x80000003 0x1 0x80000043 0x1>;
> > > +			gpio-controller;
> > > +			#gpio-cells = <2>;
> > > +		};
> > 
> > These are all just single bytes. My guess is that there is
> > actually one IP block that contains all the gpios, not
> > four identical blocks.
> 
> These is a 8-bit registers with different properties, which parses in the
> GPIO driver depending of alias (GPIO count, inverted logic etc.).

The alias should not influence the interpretation of the registers.
If each byte in there behaves differently, it would be better to
have separate "compatible" strings instead. I still don't see why
you can't just have one device node for all the registers and
then create multiple gpio_chip instances from that though.

	Arnd
--
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

Re: [PATCH 3/3] ARM: dts: clps711x: Add basic Cirrus Logic CDB89712 Development board

[Alexander Shiyan]

On Fri, 13 May 2016 15:41:11 +0200
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:

> On Friday 13 May 2016 15:30:23 Alexander Shiyan wrote:
> > Hello.
> > 
> > > Пятница, 13 мая 2016, 15:00 +03:00 от Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
> > > 
> > > On Friday 13 May 2016 14:26:28 Alexander Shiyan wrote:
> > ...
...
> > > > +		clks: clks@80000000 {
> > > > +			#clock-cells = <1>;
> > > > +			compatible = "cirrus,clps711x-clk";
> > > > +			reg = <0x80000000 0xc000>;
> > > > +			startup-frequency = <73728000>;
> > > > +		};
> > ...
> > > > +		intc: intc@80000000 {
> > > > +			compatible = "cirrus,clps711x-intc";
> > > > +			reg = <0x80000000 0x4000>;
> > > > +			interrupt-controller;
> > > > +			#interrupt-cells = <1>;
> > > > +		};
> > > 
> > > Better make the register ranges non-overlapping. This appears
> > > to start at the same place as the 'clks' node.
> > 
> > CLK driver uses:
> > #define CLPS711X_SYSCON1 (0x0100)
> > #define CLPS711X_SYSCON2 (0x1100)
> > ...
> > #define CLPS711X_PLLR (0xa5a8)
> > 
> > IRQCHIP driver uses:
> > #define CLPS711X_INTSR1 (0x0240)
> > ...
> > #define CLPS711X_INTSR2 (0x1240)
> > ...
> > #define CLPS711X_INTMR3 (0x2280)
> > 
> > So there is no way to do any else.
> 
> It sounds like what you have is a large system controller that
> has registers everywhere. Could you use syscon to access the
> individual registers instead?

There are several problems:
- The syscon driver must be initialized before any other devices,
  including the interrupt controller and clk/clocksource subsystems.
- A slight performance impact will be observed in the interrupt handler.
- SYSCON (regmap) is designed to use with equal the width of registers,
  while we need to use different widths (8/16/32) for some subsystems.

-- 
Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
--
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