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

[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

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

[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

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

[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

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

[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

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

[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

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

[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

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

[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

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

[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