On Tue, 2015-05-05 at 11:52 -0400, Oleksandr G Zhadan wrote:
> New QorIQ p1020 based board support from Arcturus Networks Inc.
> http://www.arcturusnetworks.com/products/ucp1020/
>
> Signed-off-by: Michael Durrant <mdurr...@arcturusnetworks.com>
> Signed-off-by: Oleksandr G Zhadan <ol...@arcturusnetworks.com>
> ---
> Documentation/devicetree/bindings/pci/fsl,pci.txt | 2 +-
> .../devicetree/bindings/powerpc/arcturus/board.txt | 149 ++
> .../devicetree/bindings/powerpc/arcturus/ecm.txt | 64 +
> Documentation/devicetree/bindings/usb/fsl-usb.txt | 2 +-
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> arch/powerpc/boot/dts/fsl/ucp1020som-post.dtsi | 179 ++
> arch/powerpc/boot/dts/fsl/ucp1020som-pre.dtsi | 70 +
> arch/powerpc/boot/dts/ucp1020_32b.dts | 88 +
> arch/powerpc/boot/dts/ucp1020_32b.dtsi | 174 ++
> arch/powerpc/configs/ucp1020_defconfig | 2731
> ++++++++++++++++++++
> arch/powerpc/platforms/85xx/Kconfig | 7 +
> arch/powerpc/platforms/85xx/Makefile | 1 +
> arch/powerpc/platforms/85xx/ucp1020_som.c | 100 +
> 13 files changed, 3566 insertions(+), 2 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/powerpc/arcturus/board.txt
> create mode 100644 Documentation/devicetree/bindings/powerpc/arcturus/ecm.txt
> create mode 100644 arch/powerpc/boot/dts/fsl/ucp1020som-post.dtsi
> create mode 100644 arch/powerpc/boot/dts/fsl/ucp1020som-pre.dtsi
> create mode 100644 arch/powerpc/boot/dts/ucp1020_32b.dts
> create mode 100644 arch/powerpc/boot/dts/ucp1020_32b.dtsi
> create mode 100644 arch/powerpc/configs/ucp1020_defconfig
> create mode 100644 arch/powerpc/platforms/85xx/ucp1020_som.c
>
> diff --git a/Documentation/devicetree/bindings/pci/fsl,pci.txt
> b/Documentation/devicetree/bindings/pci/fsl,pci.txt
> index d8ac4a7..298a5e6 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,pci.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,pci.txt
> @@ -20,7 +20,7 @@ Example:
> #interrupt-cells = <1>;
> #size-cells = <2>;
> #address-cells = <3>;
> - compatible = "fsl,mpc8540-pcix", "fsl,mpc8540-pci";
> + compatible = "fsl,mpc8540-pcix", "fsl,mpc8540-pci",
> "fsl,mpc8548-pcie";
> device_type = "pci";
> ...
> ...
> diff --git a/Documentation/devicetree/bindings/powerpc/arcturus/board.txt
> b/Documentation/devicetree/bindings/powerpc/arcturus/board.txt
> new file mode 100644
> index 0000000..54e9765
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/powerpc/arcturus/board.txt
> @@ -0,0 +1,149 @@
> +UCP1020 module Tree Bindings
> +----------------------------
> +
> +Copyright 2013-2015 Arcturus Networks, Inc.
> +
> +QorIQ p1020 based board
> +http://www.arcturusnetworks.com/products/ucp1020/
> +-------------------------------------------------
> +
> +Root Module
> +
> +Properties:
> +- model: "arcturus,uCP1020"
> +- compatible: "arcturus,uCP1020"
> +- SN: "1234567890-1234"
> +
> +/ {
> + model = "arcturus,uCP1020";
> + compatible = "arcturus,uCP1020", "fsl,P1020";
> + SN = "1234567890-1234";
> + ...
> + }
Drop the "fsl,P1020" compatible. Top-level compatible strings describe
the whole board.
SN is a bad property name. Call it something like "arcturus,serial#",
and define what it actually means rather than just giving an example.
> +-------------------------------------------------
> +
> +P1020 SPI controller
> +
> +Properties:
> +- compatible: "spansion,s25fl008k", "winbond,w25q80bl"
> +
> +Example:
> + spi@7000 {
> + flash@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "spansion,s25fl008k", "winbond,w25q80bl";
> + reg = <0>;
> + spi-max-frequency = <40000000>; /* input clock */
> + ...
> + };
This isn't describing the controller, but rather a SPI chip attached to
the controller. This also doesn't seem like the right place for random
SPI chips.
If all you're specifying is the compatible, maybe create a
spi/trivial-devices.txt similar to i2c/trivial-devices.txt? Or
something specific to SPI flash chips to describe the partition
specification, though I generally recommend against describing
partitions in the device tree -- especially if this is a developer board
rather than something fixed-purpose where the partitioning is not going
to change based on user requirements.
> +-------------------------------------------------
> +
> +Chipselect/Local Bus
> +
> +Properties:
> +- #address-cells: <2>.
> +- #size-cells: <1>.
> +- compatible: "fsl,p1020-elbc", "fsl,elbc",
> "simple-bus","fsl,p1020-immr"
> +- interrupts: interrupts to report localbus events.
> +
> +Example:
> +
> +&lbc {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + compatible = "fsl,p1020-elbc", "fsl,elbc", "simple-bus";
> + interrupts = <19 2 0 0>;
> +};
There's already a binding for elbc -- and the elbc node certainly should
not claim compatibility with "fsl,p1020-immr".
> +-------------------------------------------------
> +
> +Freescale L2 Cache Controller
> +
> +L2 cache is present in Freescale's QorIQ and QorIQ Qonverge platforms.
> +The cache bindings explained below are ePAPR compliant
> +
> +Properties:
> +
> +- compatible : Should include "fsl,chip-l2-cache-controller" and "cache"
> + where chip is the processor (bsc9132, npc8572 etc.)
> +- reg : Address and size of L2 cache controller registers
> +- cache-size : Size of the entire L2 cache
> +- interrupts : Error interrupt of L2 controller
> +- cache-line-size : Size of L2 cache lines
> +
> +Example:
> +
> + L2: l2-cache-controller@20000 {
> + compatible = "fsl,p1020-l2-cache-controller", "cache";
> + reg = <0x20000 0x1000>;
> + cache-line-size = <32>; // 32 bytes
> + cache-size = <0x40000>; // L2,256K
> + interrupts = <16 2 1 0>;
> + };
This is copied and pasted from the existing binding. Ignore
checkpatch's inability to understand that "fsl,chip-l2-cache-controller"
includes "fsl,p1020-l2-cache-controller".
Likewise for the other bindings. If you're going to have a binding
document for your board, it should only talk about things that are
specific to your board.
> diff --git a/Documentation/devicetree/bindings/usb/fsl-usb.txt
> b/Documentation/devicetree/bindings/usb/fsl-usb.txt
> index 4779c02..a2663ad 100644
> --- a/Documentation/devicetree/bindings/usb/fsl-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/fsl-usb.txt
> @@ -57,7 +57,7 @@ Example multi port host USB controller device node :
>
> Example dual role USB controller device node :
> usb@23000 {
> - compatible = "fsl-usb2-dr";
> + compatible = "fsl-usb2-dr", "fsl-usb2-dr-v1.6";
> reg = <23000 1000>;
> #address-cells = <1>;
> #size-cells = <0>;
More specific compatibles come first.
> diff --git a/arch/powerpc/boot/dts/fsl/ucp1020som-post.dtsi
> b/arch/powerpc/boot/dts/fsl/ucp1020som-post.dtsi
> new file mode 100644
> index 0000000..930a6e3
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/fsl/ucp1020som-post.dtsi
Why can't you use p1020si-post.dtsi? The "si" means "silicon" -- it's
meant to be included by all p1020 boards.
> diff --git a/arch/powerpc/boot/dts/ucp1020_32b.dts
> b/arch/powerpc/boot/dts/ucp1020_32b.dts
> new file mode 100644
> index 0000000..4a8358c
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/ucp1020_32b.dts
> @@ -0,0 +1,88 @@
> +/*
> + * uCP1020 Tree Source (32-bit address map)
Do you plan on supporting both 32 and 36 bits? If not, leave off the
"_32b".
> diff --git a/arch/powerpc/configs/ucp1020_defconfig
> b/arch/powerpc/configs/ucp1020_defconfig
> new file mode 100644
> index 0000000..62f99aa
> --- /dev/null
> +++ b/arch/powerpc/configs/ucp1020_defconfig
Please explain why your board needs its own defconfig.
Plus, all defconfigs need to be run through savedefconfig to trim
everything that doesn't change a default.
> diff --git a/arch/powerpc/platforms/85xx/ucp1020_som.c
> b/arch/powerpc/platforms/85xx/ucp1020_som.c
> new file mode 100644
> index 0000000..aa98d31
> --- /dev/null
> +++ b/arch/powerpc/platforms/85xx/ucp1020_som.c
> @@ -0,0 +1,100 @@
> +/*
> + * Arcturus Networks Inc. uCP1020 system on module Setup
> + *
> + * Copyright 2014-2015 Arcturus Networks Inc.
> + *
> + * by Oleksandr G Zhadan & Michael Durrant (www.ArcturusNetworks.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 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/stddef.h>
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/kdev_t.h>
> +#include <linux/delay.h>
> +#include <linux/seq_file.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/time.h>
> +#include <asm/machdep.h>
> +#include <asm/pci-bridge.h>
> +#include <mm/mmu_decl.h>
> +#include <asm/prom.h>
> +#include <asm/udbg.h>
> +#include <asm/mpic.h>
> +#include <asm/fsl_guts.h>
> +
> +#include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
> +#include "smp.h"
> +
> +#include "mpc85xx.h"
You don't need all of these headers.
> +
> +void __init ucp1020_som_pic_init(void)
> +{
> + struct mpic *mpic;
> + unsigned long root = of_get_flat_dt_root();
> +
> + if (of_flat_dt_is_compatible(root, "fsl,MPC85XXRDB-CAMP")) {
> + mpic = mpic_alloc(NULL, 0, MPIC_NO_RESET |
> + MPIC_BIG_ENDIAN |
> + MPIC_SINGLE_DEST_CPU, 0, 256, " OpenPIC ");
> + } else {
> + mpic = mpic_alloc(NULL, 0,
> + MPIC_BIG_ENDIAN |
> + MPIC_SINGLE_DEST_CPU, 0, 256, " OpenPIC ");
> + }
Why do you have fsl,MPC85XXRDB-CAMP in here?
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
