Re: [RFC PATCH v1 2/2] powerpc/86xx: Introduce and use common dtsi

2016-03-01 Thread Scott Wood 
On Tue, 2016-03-01 at 15:55 +0100, Alessio Igor Bogani wrote:
> Scott,
> 
> On 29 February 2016 at 23:38, Scott Wood  wrote:
> [...]
> > 
> > > + {
> > > + compatible = "fsl,mpc8641-pcie";
> > > + device_type = "pci";
> > > + #size-cells = <2>;
> > > + #address-cells = <3>;
> > > + bus-range = <0x0 0xff>;
> > > + clock-frequency = <>;
> > 
> > The clock frequency of PCI Express is not 33 MHz.
> 
> A lot of dtsi files into fsl directory have got that value!

Well, they shouldn't. :-P

This isn't the first time I've told someone to take that out.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v1 2/2] powerpc/86xx: Introduce and use common dtsi

2016-03-01 Thread Alessio Igor Bogani 
Scott,

On 29 February 2016 at 23:38, Scott Wood  wrote:
[...]
> Could you post a diff of what the decompiled trees look like before and after
> this change (e.g. interrupts went from 2-cell to 4-cell)?  It is very hard to
> review in this form.  Or better, like the config change, have a commit that
> first makes changes to what the unified trees will be, and then a second
> commit that generates the same output using includes.

Yes I'll surely do it.

>> +/include/ "qoriq-mpic.dtsi"
>> + pic@4 {
>> + compatible = "chrp,open-pic";
>> + };
>
> This is removing the fsl,mpic compatible, which is required for 4-cell
> interrupt specifiers.

Ok.

>> + {
>> + compatible = "fsl,mpc8641-pcie";
>> + device_type = "pci";
>> + #size-cells = <2>;
>> + #address-cells = <3>;
>> + bus-range = <0x0 0xff>;
>> + clock-frequency = <>;
>
> The clock frequency of PCI Express is not 33 MHz.

A lot of dtsi files into fsl directory have got that value!

>> diff --git a/arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi
[...]
>> +/dts-v1/;
>> +
>> +/ {
>> + compatible = "fsl,MPC8641";
>
> This compatible is pointless -- it will be overwritten by the board
> compatible.

Ok.

Thanks for have reviewed my patches.

Ciao,
Alessio
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v1 2/2] powerpc/86xx: Introduce and use common dtsi

2016-02-29 Thread Scott Wood 
On Thu, 2016-02-25 at 11:57 +0100, Alessio Igor Bogani wrote:
> Signed-off-by: Alessio Igor Bogani 
> ---
>  arch/powerpc/boot/dts/fsl/gef_ppc9a.dts| 284 +++---
>  arch/powerpc/boot/dts/fsl/gef_sbc310.dts   | 276 +++--
>  arch/powerpc/boot/dts/fsl/gef_sbc610.dts   | 284 +++---
>  arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts | 318 --
> ---
>  arch/powerpc/boot/dts/fsl/mpc8641_hpcn_36b.dts | 317 --
> --
>  arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi  | 124 ++
>  arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi   |  59 +
>  arch/powerpc/boot/dts/fsl/sbc8641d.dts | 314 +++---
> --
>  8 files changed, 400 insertions(+), 1576 deletions(-)
>  create mode 100644 arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi
>  create mode 100644 arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi

Could you post a diff of what the decompiled trees look like before and after
this change (e.g. interrupts went from 2-cell to 4-cell)?  It is very hard to
review in this form.  Or better, like the config change, have a commit that
first makes changes to what the unified trees will be, and then a second
commit that generates the same output using includes.

> +/include/ "qoriq-mpic.dtsi"
> + pic@4 {
> + compatible = "chrp,open-pic";
> + };

This is removing the fsl,mpic compatible, which is required for 4-cell
interrupt specifiers.

> + {
> + compatible = "fsl,mpc8641-pcie";
> + device_type = "pci";
> + #size-cells = <2>;
> + #address-cells = <3>;
> + bus-range = <0x0 0xff>;
> + clock-frequency = <>;

The clock frequency of PCI Express is not 33 MHz.


> diff --git a/arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi
> b/arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi
> new file mode 100644
> index 000..026fa58
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi
> @@ -0,0 +1,59 @@
> +/*
> + * MPC8641 Silicon/SoC Device Tree Source (pre include)
> + *
> + * Copyright 2016 Elettra-Sincrotrone Trieste S.C.p.A.
> + *
> + * 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.
> + *
> + */
> +
> +/dts-v1/;
> +
> +/ {
> + compatible = "fsl,MPC8641";

This compatible is pointless -- it will be overwritten by the board
compatible.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v1 2/2] powerpc/86xx: Introduce and use common dtsi

2016-02-29 Thread Alessio Igor Bogani 
Hi,

On 25 February 2016 at 11:57, Alessio Igor Bogani
 wrote:
> Signed-off-by: Alessio Igor Bogani 
> ---
>  arch/powerpc/boot/dts/fsl/gef_ppc9a.dts| 284 +++---
>  arch/powerpc/boot/dts/fsl/gef_sbc310.dts   | 276 +++--
>  arch/powerpc/boot/dts/fsl/gef_sbc610.dts   | 284 +++---
>  arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts | 318 
> -
>  arch/powerpc/boot/dts/fsl/mpc8641_hpcn_36b.dts | 317 
>  arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi  | 124 ++
>  arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi   |  59 +
>  arch/powerpc/boot/dts/fsl/sbc8641d.dts | 314 +++-
>  8 files changed, 400 insertions(+), 1576 deletions(-)
>  create mode 100644 arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi
>  create mode 100644 arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi
[...]

Any comments?

Thanks!

Ciao,
Alessio
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[RFC PATCH v1 2/2] powerpc/86xx: Introduce and use common dtsi

2016-02-25 Thread Alessio Igor Bogani 
Signed-off-by: Alessio Igor Bogani 
---
 arch/powerpc/boot/dts/fsl/gef_ppc9a.dts| 284 +++---
 arch/powerpc/boot/dts/fsl/gef_sbc310.dts   | 276 +++--
 arch/powerpc/boot/dts/fsl/gef_sbc610.dts   | 284 +++---
 arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts | 318 -
 arch/powerpc/boot/dts/fsl/mpc8641_hpcn_36b.dts | 317 
 arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi  | 124 ++
 arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi   |  59 +
 arch/powerpc/boot/dts/fsl/sbc8641d.dts | 314 +++-
 8 files changed, 400 insertions(+), 1576 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi
 create mode 100644 arch/powerpc/boot/dts/fsl/mpc8641si-pre.dtsi

diff --git a/arch/powerpc/boot/dts/fsl/gef_ppc9a.dts 
b/arch/powerpc/boot/dts/fsl/gef_ppc9a.dts
index 83eb0fd..0c11623 100644
--- a/arch/powerpc/boot/dts/fsl/gef_ppc9a.dts
+++ b/arch/powerpc/boot/dts/fsl/gef_ppc9a.dts
@@ -18,62 +18,19 @@
  * Compiled with dtc -I dts -O dtb -o gef_ppc9a.dtb gef_ppc9a.dts
  */
 
-/dts-v1/;
+/include/ "mpc8641si-pre.dtsi"
 
 / {
model = "GEF_PPC9A";
compatible = "gef,ppc9a";
-   #address-cells = <1>;
-   #size-cells = <1>;
-
-   aliases {
-   ethernet0 = 
-   ethernet1 = 
-   serial0 = 
-   serial1 = 
-   pci0 = 
-   };
-
-   cpus {
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   PowerPC,8641@0 {
-   device_type = "cpu";
-   reg = <0>;
-   d-cache-line-size = <32>;   // 32 bytes
-   i-cache-line-size = <32>;   // 32 bytes
-   d-cache-size = <32768>; // L1, 32K
-   i-cache-size = <32768>; // L1, 32K
-   timebase-frequency = <0>;   // From uboot
-   bus-frequency = <0>;// From uboot
-   clock-frequency = <0>;  // From uboot
-   };
-   PowerPC,8641@1 {
-   device_type = "cpu";
-   reg = <1>;
-   d-cache-line-size = <32>;   // 32 bytes
-   i-cache-line-size = <32>;   // 32 bytes
-   d-cache-size = <32768>; // L1, 32K
-   i-cache-size = <32768>; // L1, 32K
-   timebase-frequency = <0>;   // From uboot
-   bus-frequency = <0>;// From uboot
-   clock-frequency = <0>;  // From uboot
-   };
-   };
 
memory {
device_type = "memory";
reg = <0x0 0x4000>; // set by uboot
};
 
-   localbus@fef05000 {
-   #address-cells = <2>;
-   #size-cells = <1>;
-   compatible = "fsl,mpc8641-localbus", "simple-bus";
+   lbc: localbus@fef05000 {
reg = <0xfef05000 0x1000>;
-   interrupts = <19 2>;
-   interrupt-parent = <>;
 
ranges = <0 0 0xff00 0x0100 // 16MB Boot flash
  1 0 0xe800 0x0800 // Paged Flash 0
@@ -133,7 +90,7 @@
compatible = "gef,ppc9a-fpga-wdt", "gef,fpga-wdt-1.00",
"gef,fpga-wdt";
reg = <0x4 0x2000 0x8>;
-   interrupts = <0x1a 0x4>;
+   interrupts = <0x1a 0x4 0 0>;
interrupt-parent = <_pic>;
};
/* Second watchdog available, driver currently supports one.
@@ -141,7 +98,7 @@
compatible = "gef,ppc9a-fpga-wdt", "gef,fpga-wdt-1.00",
"gef,fpga-wdt";
reg = <0x4 0x2010 0x8>;
-   interrupts = <0x1b 0x4>;
+   interrupts = <0x1b 0x4 0 0>;
interrupt-parent = <_pic>;
};
*/
@@ -150,10 +107,7 @@
interrupt-controller;
compatible = "gef,ppc9a-fpga-pic", "gef,fpga-pic-1.00";
reg = <0x4 0x4000 0x20>;
-   interrupts = <0x8
- 0x9>;
-   interrupt-parent = <>;
-
+   interrupts = <0x8 0x9 0 0>;
};
gef_gpio: gpio@7,14000 {
#gpio-cells = <2>;
@@ -163,37 +117,10 @@
};
};
 
-   soc@fef0 {
-   #address-cells = <1>;
-   #size-cells = <1>;
-   #interrupt-cells = <2>;
-   device_type = "soc";
-   compatible =