Hi Scott,

Thanks for fast response, please see inline.

On 05/06/2015 11:22 PM, Scott Wood wrote:
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.


OK, will fix.

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



Mostly in all Documentation/devicetree/bindings/ I tried to satisfy checkpatch script as simple as possible. And for me as well it looks reasonable to create spi/trivial-devices.txt file and I will.

+-------------------------------------------------
+
+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".



to satisfy checkpatch script.

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


to satisfy checkpatch script.


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.


OK.

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.


Yes, silicon is the same, but p1020 boards using all 3 etsec ethernet controllers. Our board using only 2: etsec1 and etsec3. And to avoid using eth0,eth2, and to use eth0,eth1 we make this extra ucp1020som-*.

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


Good point - will rename.

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.


Because, it's our own board and it has some specific to board definitions like CONFIG_DEFAULT_HOSTNAME and some specific to product definitions.

If I can do it in some other way could you please give me some example if it's possible.


Plus, all defconfigs need to be run through savedefconfig to trim
everything that doesn't change a default.


Sorry, forgot. Will do.

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?


ups, didn't notice(cut and paste) will fix.


Probably will delay with patches fixes until May 21 - I'll be out of office.


Oleks

-Scott





--
Oleksandr Zhadan
Arcturus Networks
T 416 621-0125 x235
F 416 621-0190
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to