| Message ID | 1463138788-5390-4-git-send-email-shc_work@mail.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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. ---
On Friday 13 May 2016 15:30:23 Alexander Shiyan wrote: > 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. 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
On Fri, 13 May 2016 15:41:11 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 13 May 2016 15:30:23 Alexander Shiyan wrote: > > Hello. > > > > > ???????, 13 ??? 2016, 15:00 +03:00 ?? Arnd Bergmann <arnd@arndb.de>: > > > > > > 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.
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>; + }; + +};
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@mail.ru> --- 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