| Message ID | 1371040448-28742-3-git-send-email-jonas.jensen@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 12, 2013 at 02:34:07PM +0200, Jonas Jensen wrote: > > Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com> Hi, You should add a bindings description for the MOXA SoC platforms, similar to how others do it, under Documentation/devicetree/bindings/arm/. Same goes for other device bindings below -- they all need to be added to the bindings documentation. You might be better off removing some of them until you post and merge the corresponding drivers. > --- > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/moxart-uc7112lx.dts | 89 +++++++++++++++++++++++++++++++++ > arch/arm/boot/dts/moxart.dtsi | 84 +++++++++++++++++++++++++++++++ > 3 files changed, 174 insertions(+), 0 deletions(-) > create mode 100644 arch/arm/boot/dts/moxart-uc7112lx.dts > create mode 100644 arch/arm/boot/dts/moxart.dtsi > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 0d1e98b..059e6d3 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -217,6 +217,7 @@ dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \ > wm8750-apc8750.dtb \ > wm8850-w70v2.dtb > dtb-$(CONFIG_ARCH_ZYNQ) += zynq-zc702.dtb > +dtb-$(CONFIG_ARCH_MOXART) += moxart-uc7112lx.dtb > > targets += dtbs > targets += $(dtb-y) > diff --git a/arch/arm/boot/dts/moxart-uc7112lx.dts b/arch/arm/boot/dts/moxart-uc7112lx.dts > new file mode 100644 > index 0000000..c2bb7dc > --- /dev/null > +++ b/arch/arm/boot/dts/moxart-uc7112lx.dts > @@ -0,0 +1,89 @@ > +/* moxart-uc7112lx.dts - Device Tree file for MOXA UC-7112-LX > + * > + * Copyright (C) 2013 Jonas Jensen <jonas.jensen@gmail.com> > + * > + * Licensed under GPLv2 or later. */ */ on new line > + > +/dts-v1/; > +/include/ "moxart.dtsi" > + > +/ { > + model = "MOXA UC-7112-LX"; > + compatible = "moxa,moxart-uc-7112-lx"; Is there a generic board design / eval board that you can have as a fallback compatible value? That way you won't have to add every board to the table in the c file. > + > + memory { > + reg = <0x00000000 0x02000000>; > + }; > + > + flash@80000000,0 { > + /* JS28F128 J3D75 A9087684 - Numonyx Embedded Flash Memory (J3 v. D) */ > + compatible = "numonyx,js28f128", "cfi-flash"; > + reg = <0x80000000 0x01000000>; > + bank-width = <2>; > + #address-cells = <1>; > + #size-cells = <1>; > + partition@0 { > + label = "bootloader"; > + reg = <0x00000000 0x00040000>; > + }; > + partition@40000 { > + label = "linux kernel"; > + reg = <0x00040000 0x001C0000>; > + }; > + partition@200000 { > + label = "root filesystem"; > + reg = <0x00200000 0x00800000>; > + }; > + partition@a00000 { > + label = "user filesystem"; > + reg = <0x00a00000 0x00600000>; > + }; > + }; > + > + mmc@98e00000 { > + compatible = "moxa,moxart-mmc"; > + reg = <0x98e00000 0x00001000>; > + interrupts = <5 0>; > + clock-names = "sys_clk"; > + clocks = <&sys_clk>; > + }; > + > + mxser@98200040 { > + compatible = "moxa,moxart-mxser"; > + reg = <0x98200040 0x00000080>, /* UART "3" base */ > + <0x982000e4 0x00000080>, /* UART mode base */ > + <0x982000c0 0x00000020>; /* UART interrupt vector */ Are there other registers for other devices inbetween, or could you just do one large memory area here? > + interrupts = <31 1>; > + }; > + > + mac0: mac@90900000 { > + compatible = "moxa,moxart-mac0"; > + reg = <0x90900000 0x1000>, > + <0x80000050 0x5>; /* MAC address stored on flash */ This is a pretty unusal way to indicate mac address location. While I suppose it works, I'd like to get the device tree maintainers in the loop. ADding them to cc. Also, there should be a bindings doc for this device (and a driver) I'm also surprised that you have a 5-byte mac address. 6 bytes is much more common. :) Finally, are the two MACs using the same driver? if so, using the same compatible value makes more sense. Also, while we're not super-strict about line lengths, please check your tabbing and try to bring the comments in a bit. > + interrupts = <25 0>; > + }; > + > + mac1: mac@92000000 { > + compatible = "moxa,moxart-mac1"; > + reg = <0x92000000 0x1000>, > + <0x80000056 0x5>; /* MAC address stored on flash */ > + interrupts = <27 0>; > + }; > + > + uart0: uart@98200000 { > + compatible = "ns16550a"; > + reg = <0x98200000 0x20>; > + interrupts = <31 0>; > + reg-shift = <2>; > + reg-io-width = <4>; > + clock-frequency = <14745600>; > + status = "okay"; > + }; > + > + chosen { > + /* uncomment to use on board flash root > + bootargs = "console=ttyS0,115200n8 root=/dev/mtdblock2 rootfstype=jffs2 rw"; > + */ Just leave that out, I'd say. > + bootargs = "console=ttyS0,115200n8 root=/dev/mmcblk0p1 rw rootwait"; > + }; > +}; > diff --git a/arch/arm/boot/dts/moxart.dtsi b/arch/arm/boot/dts/moxart.dtsi > new file mode 100644 > index 0000000..debedb7 > --- /dev/null > +++ b/arch/arm/boot/dts/moxart.dtsi > @@ -0,0 +1,84 @@ > +/* moxart.dtsi - Device Tree Include file for MOXA ART family SoC > + * > + * Copyright (C) 2013 Jonas Jensen <jonas.jensen@gmail.com> > + * > + * Licensed under GPLv2 or later. */ > + > +/include/ "skeleton.dtsi" > + > +/ { > + interrupt-parent = <&intc>; > + > + cpus { > + cpu@0 { > + compatible = "faraday,fa526"; There are a couple of more properties required here. See recent postings on cpu bindings. > + }; > + }; > + > + clocks { > + #address-cells = <1>; > + #size-cells = <0>; > + > + osc: oscillator { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <24000000>; > + }; > + }; > + > + soc { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x90000000 0x10000000>; > + ranges; > + > + intc: interrupt-controller@98800000 { > + compatible = "moxa,moxart-interrupt-controller"; > + reg = <0x98800000 0x38>; > + interrupt-controller; > + #interrupt-cells = <2>; > + interrupt-mask = <0x00080000>; /* single register vector, interrupts 0-31, 1s signify edge */ Very long line, here and below. > + }; > + > + timer: timer@98400000 { > + compatible = "moxa,moxart-timer"; > + reg = <0x98400000 0x10>; > + interrupts = <19 1>; > + }; > + > + gpio: gpio@98700000 { > + compatible = "moxa,moxart-gpio"; > + reg = <0x98700000 0x1000>, > + <0x98100100 0x4>; /* Power Management Unit > + enable/disable pin 0-31 (32bit register) */ > + }; > + > + rtc: rtc { > + compatible = "moxa,moxart-rtc"; > + }; > + > + dma: dma@90500000 { > + compatible = "moxa,moxart-dma"; > + reg = <0x90500000 0x1000>; > + interrupts = <24 0>; > + }; > + > + watchdog: watchdog@98500000 { > + compatible = "moxa,moxart-watchdog"; > + reg = <0x98500000 0x1000>; > + }; > + > + pmu: pmu@98100000 { /* Power Management Unit */ > + compatible = "moxa,moxart-pmu"; > + reg = <0x98100000 0x34>; /* offset mul @ 0x30, val @ 0x0c (2 * 32 bit registers) */ > + clocks { > + sys_clk: sys_clk { > + #clock-cells = <0>; > + compatible = "moxa,moxart-sysclk"; > + clock-output-names = "sys_clk"; > + }; > + }; > + }; > + }; > +}; > -- > 1.7.2.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
Hi, Thanks for the replies. What isn't commented below should already be fixed. On 13 June 2013 00:49, Olof Johansson <olof@lixom.net> wrote: > Hi, > > You should add a bindings description for the MOXA SoC platforms, similar to > how others do it, under Documentation/devicetree/bindings/arm/. The following is now added under Documentation: Documentation/devicetree/bindings/arm/moxart.txt | 8 +++ .../arm/moxart/moxart-interrupt-controller.txt | 29 ++++++++++++ .../bindings/arm/moxart/moxart-timer.txt | 17 +++++++ > Same goes for other device bindings below -- they all need to be added to the > bindings documentation. You might be better off removing some of them until you > post and merge the corresponding drivers. I'll remove them for now with the intent of adding them later. Maybe when (and if) all drivers are posted and merged. >> +/dts-v1/; >> +/include/ "moxart.dtsi" >> + >> +/ { >> + model = "MOXA UC-7112-LX"; >> + compatible = "moxa,moxart-uc-7112-lx"; > > Is there a generic board design / eval board that you can have as a fallback > compatible value? That way you won't have to add every board to the table in > the c file. There isn't really a generic board but the same can be accomplished by adding "moxa,moxart" to compatible? New boards can then be added without patching arch/arm/mach-moxart/moxart.c. >> + mxser@98200040 { >> + compatible = "moxa,moxart-mxser"; >> + reg = <0x98200040 0x00000080>, /* UART "3" base */ >> + <0x982000e4 0x00000080>, /* UART mode base */ >> + <0x982000c0 0x00000020>; /* UART interrupt vector */ > > Are there other registers for other devices inbetween, or could you just do one > large memory area here? No, reg could simply be 0x98200040 to 0x982000e0. I split them out as documentation but those could be defined in the driver instead, also because it's sort of easier to assign values with of_iomap. >> + interrupts = <31 1>; >> + }; >> + >> + mac0: mac@90900000 { >> + compatible = "moxa,moxart-mac0"; >> + reg = <0x90900000 0x1000>, >> + <0x80000050 0x5>; /* MAC address stored on flash */ > > This is a pretty unusal way to indicate mac address location. While I suppose > it works, I'd like to get the device tree maintainers in the loop. ADding them > to cc. > > Also, there should be a bindings doc for this device (and a driver) I'm removing ethernet until the driver is submitted. This allows me to submit bindings doc later? > I'm also surprised that you have a 5-byte mac address. 6 bytes is much more > common. :) That _is_ surprising :) Corrected to 6 bytes. > Finally, are the two MACs using the same driver? if so, using the same > compatible value makes more sense. Yes, using the same driver is the point. I see now they can use the same value, they'll still be probed from the DT entries. Best regards, Jonas
On Fri, Jun 14, 2013 at 04:34:24PM +0200, Jonas Jensen wrote: > Hi, > > Thanks for the replies. > > What isn't commented below should already be fixed. > > On 13 June 2013 00:49, Olof Johansson <olof@lixom.net> wrote: > > Hi, > > > > You should add a bindings description for the MOXA SoC platforms, similar to > > how others do it, under Documentation/devicetree/bindings/arm/. > > The following is now added under Documentation: > > Documentation/devicetree/bindings/arm/moxart.txt | 8 +++ > .../arm/moxart/moxart-interrupt-controller.txt | 29 ++++++++++++ > .../bindings/arm/moxart/moxart-timer.txt | 17 +++++++ > > > Same goes for other device bindings below -- they all need to be added to the > > bindings documentation. You might be better off removing some of them until you > > post and merge the corresponding drivers. > > I'll remove them for now with the intent of adding them later. Maybe > when (and if) all drivers are posted and merged. > > >> +/dts-v1/; > >> +/include/ "moxart.dtsi" > >> + > >> +/ { > >> + model = "MOXA UC-7112-LX"; > >> + compatible = "moxa,moxart-uc-7112-lx"; > > > > Is there a generic board design / eval board that you can have as a fallback > > compatible value? That way you won't have to add every board to the table in > > the c file. > > There isn't really a generic board but the same can be accomplished by > adding "moxa,moxart" to compatible? New boards can then be added > without patching arch/arm/mach-moxart/moxart.c. Sounds good. See comment on the other email about checking for machine compatible in the init setup function too, that'll work well. > > >> + mxser@98200040 { > >> + compatible = "moxa,moxart-mxser"; > >> + reg = <0x98200040 0x00000080>, /* UART "3" base */ > >> + <0x982000e4 0x00000080>, /* UART mode base */ > >> + <0x982000c0 0x00000020>; /* UART interrupt vector */ > > > > Are there other registers for other devices inbetween, or could you just do one > > large memory area here? > > No, reg could simply be 0x98200040 to 0x982000e0. I split them out as > documentation but those could be defined in the driver instead, also > because it's sort of easier to assign values with of_iomap. I'd say just go with one register area, it's how most other drivers do it so it's most familiar to someone else coming in and reading your code to fix bugs or whatnot. > >> + interrupts = <31 1>; > >> + }; > >> + > >> + mac0: mac@90900000 { > >> + compatible = "moxa,moxart-mac0"; > >> + reg = <0x90900000 0x1000>, > >> + <0x80000050 0x5>; /* MAC address stored on flash */ > > > > This is a pretty unusal way to indicate mac address location. While I suppose > > it works, I'd like to get the device tree maintainers in the loop. ADding them > > to cc. > > > > Also, there should be a bindings doc for this device (and a driver) > > I'm removing ethernet until the driver is submitted. This allows me to > submit bindings doc later? Yep. > > > I'm also surprised that you have a 5-byte mac address. 6 bytes is much more > > common. :) > > That _is_ surprising :) Corrected to 6 bytes. :) > > Finally, are the two MACs using the same driver? if so, using the same > > compatible value makes more sense. > > Yes, using the same driver is the point. I see now they can use the > same value, they'll still be probed from the DT entries. Yeah. Let's discuss that further when the driver is ready if needed. -Olof
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 0d1e98b..059e6d3 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -217,6 +217,7 @@ dtb-$(CONFIG_ARCH_VT8500) += vt8500-bv07.dtb \ wm8750-apc8750.dtb \ wm8850-w70v2.dtb dtb-$(CONFIG_ARCH_ZYNQ) += zynq-zc702.dtb +dtb-$(CONFIG_ARCH_MOXART) += moxart-uc7112lx.dtb targets += dtbs targets += $(dtb-y) diff --git a/arch/arm/boot/dts/moxart-uc7112lx.dts b/arch/arm/boot/dts/moxart-uc7112lx.dts new file mode 100644 index 0000000..c2bb7dc --- /dev/null +++ b/arch/arm/boot/dts/moxart-uc7112lx.dts @@ -0,0 +1,89 @@ +/* moxart-uc7112lx.dts - Device Tree file for MOXA UC-7112-LX + * + * Copyright (C) 2013 Jonas Jensen <jonas.jensen@gmail.com> + * + * Licensed under GPLv2 or later. */ + +/dts-v1/; +/include/ "moxart.dtsi" + +/ { + model = "MOXA UC-7112-LX"; + compatible = "moxa,moxart-uc-7112-lx"; + + memory { + reg = <0x00000000 0x02000000>; + }; + + flash@80000000,0 { + /* JS28F128 J3D75 A9087684 - Numonyx Embedded Flash Memory (J3 v. D) */ + compatible = "numonyx,js28f128", "cfi-flash"; + reg = <0x80000000 0x01000000>; + bank-width = <2>; + #address-cells = <1>; + #size-cells = <1>; + partition@0 { + label = "bootloader"; + reg = <0x00000000 0x00040000>; + }; + partition@40000 { + label = "linux kernel"; + reg = <0x00040000 0x001C0000>; + }; + partition@200000 { + label = "root filesystem"; + reg = <0x00200000 0x00800000>; + }; + partition@a00000 { + label = "user filesystem"; + reg = <0x00a00000 0x00600000>; + }; + }; + + mmc@98e00000 { + compatible = "moxa,moxart-mmc"; + reg = <0x98e00000 0x00001000>; + interrupts = <5 0>; + clock-names = "sys_clk"; + clocks = <&sys_clk>; + }; + + mxser@98200040 { + compatible = "moxa,moxart-mxser"; + reg = <0x98200040 0x00000080>, /* UART "3" base */ + <0x982000e4 0x00000080>, /* UART mode base */ + <0x982000c0 0x00000020>; /* UART interrupt vector */ + interrupts = <31 1>; + }; + + mac0: mac@90900000 { + compatible = "moxa,moxart-mac0"; + reg = <0x90900000 0x1000>, + <0x80000050 0x5>; /* MAC address stored on flash */ + interrupts = <25 0>; + }; + + mac1: mac@92000000 { + compatible = "moxa,moxart-mac1"; + reg = <0x92000000 0x1000>, + <0x80000056 0x5>; /* MAC address stored on flash */ + interrupts = <27 0>; + }; + + uart0: uart@98200000 { + compatible = "ns16550a"; + reg = <0x98200000 0x20>; + interrupts = <31 0>; + reg-shift = <2>; + reg-io-width = <4>; + clock-frequency = <14745600>; + status = "okay"; + }; + + chosen { + /* uncomment to use on board flash root + bootargs = "console=ttyS0,115200n8 root=/dev/mtdblock2 rootfstype=jffs2 rw"; + */ + bootargs = "console=ttyS0,115200n8 root=/dev/mmcblk0p1 rw rootwait"; + }; +}; diff --git a/arch/arm/boot/dts/moxart.dtsi b/arch/arm/boot/dts/moxart.dtsi new file mode 100644 index 0000000..debedb7 --- /dev/null +++ b/arch/arm/boot/dts/moxart.dtsi @@ -0,0 +1,84 @@ +/* moxart.dtsi - Device Tree Include file for MOXA ART family SoC + * + * Copyright (C) 2013 Jonas Jensen <jonas.jensen@gmail.com> + * + * Licensed under GPLv2 or later. */ + +/include/ "skeleton.dtsi" + +/ { + interrupt-parent = <&intc>; + + cpus { + cpu@0 { + compatible = "faraday,fa526"; + }; + }; + + clocks { + #address-cells = <1>; + #size-cells = <0>; + + osc: oscillator { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <24000000>; + }; + }; + + soc { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + reg = <0x90000000 0x10000000>; + ranges; + + intc: interrupt-controller@98800000 { + compatible = "moxa,moxart-interrupt-controller"; + reg = <0x98800000 0x38>; + interrupt-controller; + #interrupt-cells = <2>; + interrupt-mask = <0x00080000>; /* single register vector, interrupts 0-31, 1s signify edge */ + }; + + timer: timer@98400000 { + compatible = "moxa,moxart-timer"; + reg = <0x98400000 0x10>; + interrupts = <19 1>; + }; + + gpio: gpio@98700000 { + compatible = "moxa,moxart-gpio"; + reg = <0x98700000 0x1000>, + <0x98100100 0x4>; /* Power Management Unit + enable/disable pin 0-31 (32bit register) */ + }; + + rtc: rtc { + compatible = "moxa,moxart-rtc"; + }; + + dma: dma@90500000 { + compatible = "moxa,moxart-dma"; + reg = <0x90500000 0x1000>; + interrupts = <24 0>; + }; + + watchdog: watchdog@98500000 { + compatible = "moxa,moxart-watchdog"; + reg = <0x98500000 0x1000>; + }; + + pmu: pmu@98100000 { /* Power Management Unit */ + compatible = "moxa,moxart-pmu"; + reg = <0x98100000 0x34>; /* offset mul @ 0x30, val @ 0x0c (2 * 32 bit registers) */ + clocks { + sys_clk: sys_clk { + #clock-cells = <0>; + compatible = "moxa,moxart-sysclk"; + clock-output-names = "sys_clk"; + }; + }; + }; + }; +};
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com> --- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/moxart-uc7112lx.dts | 89 +++++++++++++++++++++++++++++++++ arch/arm/boot/dts/moxart.dtsi | 84 +++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 0 deletions(-) create mode 100644 arch/arm/boot/dts/moxart-uc7112lx.dts create mode 100644 arch/arm/boot/dts/moxart.dtsi