Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port

Kumar Gala
2007-09-11T15:55:59Z

On Sep 11, 2007, at 9:17 AM, Scott Wood wrote:

> On Tue, Sep 11, 2007 at 01:29:18AM -0500, Kumar Gala wrote:
>> diff --git a/arch/powerpc/boot/dts/mpc8572ds.dts b/arch/powerpc/
>> boot/dts/mpc8572ds.dts
>> new file mode 100644
>> index 0000000..9d23561
>> --- /dev/null
>> +++ b/arch/powerpc/boot/dts/mpc8572ds.dts
>> @@ -0,0 +1,378 @@
>> +/*
>> + * MPC8572 DS Device Tree Source
>> + *
>> + * Copyright 2007 Freescale Semiconductor Inc.
>> + *
>> + * 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.
>> + */
>> +
>> +/ {
>> + model = "MPC8572DS";
>> + compatible = "MPC8572DS", "MPC85xxDS";
>
> "fsl," prefix on compatible.
>
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + cpus {
>> + #cpus = <1>;
>
> Where is #cpus defined or used? I don't see it used in any other
> trees, either.

will kill.

>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + PowerPC,8572@0 {
>> + device_type = "cpu";
>> + reg = <0>;
>> + d-cache-line-size = <20>; // 32 bytes
>> + i-cache-line-size = <20>; // 32 bytes
>> + d-cache-size = <8000>; // L1, 32K
>> + i-cache-size = <8000>; // L1, 32K
>> + timebase-frequency = <0>;
>> + bus-frequency = <0>;
>> + clock-frequency = <0>;
>> + 32-bit;
>
> Where is 32-bit; defined or used?

its a copy paste error. Will kill 32-bit on other platforms.

>
>> + soc8572@ffe00000 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + #interrupt-cells = <2>;
>> + device_type = "soc";
>> + ranges = <00000000 ffe00000 00100000
>> + 80000000 80000000 20000000
>> + a0000000 a0000000 20000000
>> + c0000000 c0000000 20000000
>> + ffc00000 ffc00000 00010000
>> + ffc10000 ffc10000 00010000
>> + ffc20000 ffc20000 00010000>;
>> +
>> + reg = <ffe00000 00001000>; // CCSRBAR 1M
>
> Comment doesn't match what's actually in reg.

will fix.

>
>> + bus-frequency = <0>; // Filled out by uboot.
>> +
>> + memory-controller@2000 {
>> + compatible = "fsl,8572-memory-controller";
>
> Is it compatible with any other 85xx memory controller?

maybe, but I don't want to get into that just yet.

>
>> + reg = <2000 1000>;
>> + interrupt-parent = <&mpic>;
>> + interrupts = <12 2>;
>> + };
>> +
>> + l2-cache-controller@20000 {
>> + compatible = "fsl,8572-l2-cache-controller";
>> + reg = <20000 1000>;
>> + cache-line-size = <20>; // 32 bytes
>> + cache-size = <80000>; // L2, 512K
>> + interrupt-parent = <&mpic>;
>> + interrupts = <10 2>;
>> + };
>
> Should this node be referenced by an l2-cache property in the cpu
> node?

No, its a front side cache.

>
>> + pcie@8000 {
>> + compatible = "fsl,mpc8641-pcie";
>
> Should probably be "fsl,mpc8572-pcie", "fsl,mpc8641-pcie".

this should "fsl,mpc8548-pcie" to match other 85xx pcie.

>
>> + device_type = "pci";
>> + #interrupt-cells = <1>;
>> + #size-cells = <2>;
>> + #address-cells = <3>;
>> + reg = <8000 1000>;
>> + bus-range = <0 ff>;
>> + ranges = <02000000 0 80000000 80000000 0 20000000
>> + 01000000 0 00000000 ffc00000 0 00010000>;
>
> No prefetchable mem space?

we haven't normally provided prefetch on 85xx/86xx.. will deal with
this later.

>
>> + clock-frequency = <1fca055>;
>
> Decimal would be nicer.
>
>> + pcie@9000 {
>> + compatible = "fsl,mpc8548-pcie";
>
> Why is this one 8548-compatible and the previous one 8641-compatible?

copy paste, will fix.

>
>> + mpic: pic@40000 {
>> + clock-frequency = <0>;
>> + interrupt-controller;
>> + #address-cells = <0>;
>> + #interrupt-cells = <2>;
>> + reg = <40000 40000>;
>> + built-in;
>> + compatible = "chrp,open-pic";
>> + device_type = "open-pic";
>> + big-endian;
>> + };
>
> Where is the built-in; property defined or used?

defined in chrp OF spec, not sure why we bother with it.

>
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 06d23e1..c98b867 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -374,10 +374,9 @@
>> #define PCI_DEVICE_ID_ATI_IXP400_SATA 0x4379
>> #define PCI_DEVICE_ID_ATI_IXP400_SATA2 0x437a
>> #define PCI_DEVICE_ID_ATI_IXP600_SATA 0x4380
>> -#define PCI_DEVICE_ID_ATI_IXP600_SMBUS 0x4385
>> +#define PCI_DEVICE_ID_ATI_SBX00_SMBUS 0x4385
>> #define PCI_DEVICE_ID_ATI_IXP600_IDE 0x438c
>> #define PCI_DEVICE_ID_ATI_IXP700_SATA 0x4390
>> -#define PCI_DEVICE_ID_ATI_IXP700_SMBUS 0x4395
>> #define PCI_DEVICE_ID_ATI_IXP700_IDE 0x439c
>
> What's going on here?

copied file over from an older tree.

- k