| Message ID | 1389879525-27130-1-git-send-email-valentin.longchamp@keymile.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Scott Wood |
| Headers | show |
On Thu, 2014-01-16 at 14:38 +0100, Valentin Longchamp wrote: > This patch introduces the support for Keymile's kmp204x reference > design. This design is based on Freescale's P2040/P2041 SoC. > > The peripherals used by this design are: > - SPI NOR Flash as bootloader medium > - NAND Flash with a ubi partition > - 2 PCIe busses (hosts 1 and 3) > - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) > - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt > FPGA > - 2 HW I2C busses > - last but not least, the mandatory serial port > > The patch also adds a defconfig file for this reference design and a DTS > file for the kmcoge4 board which is the first one based on this > reference design. > > To try to avoid code duplication, the support was added directly to the > corenet_generic.c file. > > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > --- > arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ > arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ > arch/powerpc/platforms/85xx/Kconfig | 14 ++ > arch/powerpc/platforms/85xx/Makefile | 1 + > arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ > 5 files changed, 463 insertions(+) > create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts > create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig > > diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts > new file mode 100644 > index 0000000..c10df6d > --- /dev/null > +++ b/arch/powerpc/boot/dts/kmcoge4.dts > @@ -0,0 +1,165 @@ > +/* > + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS > + * > + * (C) Copyright 2014 > + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com > + * > + * Copyright 2011 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. > + */ > + > +/include/ "fsl/p2041si-pre.dtsi" > + > +/ { > + model = "keymile,kmcoge4"; > + compatible = "keymile,kmp204x"; Don't put wildcards in compatible. > + soc: soc@ffe000000 { > + ranges = <0x00000000 0xf 0xfe000000 0x1000000>; > + reg = <0xf 0xfe000000 0 0x00001000>; > + spi@110000 { > + flash@0 { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "spansion,s25fl256s1"; > + reg = <0>; > + spi-max-frequency = <20000000>; /* input clock */ > + partition@u-boot { > + label = "u-boot"; > + reg = <0x00000000 0x00100000>; > + read-only; > + }; > + partition@env { > + label = "env"; > + reg = <0x00100000 0x00010000>; > + }; > + partition@envred { > + label = "envred"; > + reg = <0x00110000 0x00010000>; > + }; > + partition@fman { > + label = "fman-ucode"; > + reg = <0x00120000 0x00010000>; > + read-only; > + }; > + }; I realize it's common practice, but it would be good to get away from putting partition layouts in the dts file. Alternatives include using mtdparts on the command line, or having U-Boot put the partition info into the dtb based on the mtdparts environment variable (there is existing code for this). > + zl30343@1 { > + compatible = "gen,spidev"; Node names are supposed to be generic. Compatibles are supposed to be specific. > + lbc: localbus@ffe124000 { > + reg = <0xf 0xfe124000 0 0x1000>; > + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ > + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ > + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ > + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ > + > + nand@0,0 { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "fsl,elbc-fcm-nand"; > + reg = <0 0 0x40000>; > + > + partition@0 { > + label = "ubi0"; > + reg = <0x0 0x8000000>; > + }; > + }; > + }; No nodes for those other chipselects? > diff --git a/arch/powerpc/configs/85xx/kmp204x_defconfig b/arch/powerpc/configs/85xx/kmp204x_defconfig > new file mode 100644 > index 0000000..3bbf4fa > --- /dev/null > +++ b/arch/powerpc/configs/85xx/kmp204x_defconfig Why does this board need its own defconfig? > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > index fbd871e..8e84e1c 100644 > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > @@ -122,6 +122,7 @@ static const char * const hv_boards[] __initconst = { > NULL > }; > > +#ifdef CONFIG_CORENET_GENERIC corenet_generic.c without CONFIG_CORENET_GENERIC? > /* > * Called very early, device-tree isn't unflattened > */ > @@ -180,3 +181,54 @@ machine_arch_initcall(corenet_generic, corenet_gen_publish_devices); > #ifdef CONFIG_SWIOTLB > machine_arch_initcall(corenet_generic, swiotlb_setup_bus_notifier); > #endif > +#endif > + > +#ifdef CONFIG_KMP204X > +/* > + * Called very early, device-tree isn't unflattened > + */ > +static int __init kmp204x_generic_probe(void) > +{ > + unsigned long root = of_get_flat_dt_root(); > + > + return of_flat_dt_is_compatible(root, "keymile,kmp204x"); > +} > + > + > +/* > + * Setup the architecture > + */ > +void __init kmp204x_gen_setup_arch(void) > +{ > + mpc85xx_smp_init(); > + > + swiotlb_detect_4g(); > + > + pr_info("%s platform from Keymile\n", ppc_md.name); > +} > + > +define_machine(kmp204x) { > + .name = "kmp204x", > + .probe = kmp204x_generic_probe, > + .setup_arch = kmp204x_gen_setup_arch, > + .init_IRQ = corenet_gen_pic_init, > +#ifdef CONFIG_PCI > + .pcibios_fixup_bus = fsl_pcibios_fixup_bus, > +#endif > + .get_irq = mpic_get_coreint_irq, > + .restart = fsl_rstcr_restart, > + .calibrate_decr = generic_calibrate_decr, > + .progress = udbg_progress, > +#ifdef CONFIG_PPC64 > + .power_save = book3e_idle, > +#else > + .power_save = e500_idle, > +#endif > +}; > + > +machine_arch_initcall(kmp204x, corenet_gen_publish_devices); > + > +#ifdef CONFIG_SWIOTLB > +machine_arch_initcall(kmp204x, swiotlb_setup_bus_notifier); > +#endif > +#endif The whole point of corenet_generic.c is to avoid duplicating all of this stuff. Can't you just use corenet_generic as-is other than adding the compatible to boards[]? If not, explain why and put it in a different file. -Scott
Hi Scott, Thanks for you feedback. On 01/17/2014 12:35 AM, Scott Wood wrote: > On Thu, 2014-01-16 at 14:38 +0100, Valentin Longchamp wrote: >> This patch introduces the support for Keymile's kmp204x reference >> design. This design is based on Freescale's P2040/P2041 SoC. >> >> The peripherals used by this design are: >> - SPI NOR Flash as bootloader medium >> - NAND Flash with a ubi partition >> - 2 PCIe busses (hosts 1 and 3) >> - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) >> - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt >> FPGA >> - 2 HW I2C busses >> - last but not least, the mandatory serial port >> >> The patch also adds a defconfig file for this reference design and a DTS >> file for the kmcoge4 board which is the first one based on this >> reference design. >> >> To try to avoid code duplication, the support was added directly to the >> corenet_generic.c file. >> >> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> >> --- >> arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ >> arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ >> arch/powerpc/platforms/85xx/Kconfig | 14 ++ >> arch/powerpc/platforms/85xx/Makefile | 1 + >> arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ >> 5 files changed, 463 insertions(+) >> create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts >> create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig >> >> diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts >> new file mode 100644 >> index 0000000..c10df6d >> --- /dev/null >> +++ b/arch/powerpc/boot/dts/kmcoge4.dts >> @@ -0,0 +1,165 @@ >> +/* >> + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS >> + * >> + * (C) Copyright 2014 >> + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com >> + * >> + * Copyright 2011 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. >> + */ >> + >> +/include/ "fsl/p2041si-pre.dtsi" >> + >> +/ { >> + model = "keymile,kmcoge4"; >> + compatible = "keymile,kmp204x"; > > Don't put wildcards in compatible. Well it's a wildcard in the sense that we support both the p2040 and the p2041, but it's also the name of the plaftorm, similarly to names like '85xx' or 'tqm85xx'. > >> + soc: soc@ffe000000 { >> + ranges = <0x00000000 0xf 0xfe000000 0x1000000>; >> + reg = <0xf 0xfe000000 0 0x00001000>; >> + spi@110000 { >> + flash@0 { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + compatible = "spansion,s25fl256s1"; >> + reg = <0>; >> + spi-max-frequency = <20000000>; /* input clock */ >> + partition@u-boot { >> + label = "u-boot"; >> + reg = <0x00000000 0x00100000>; >> + read-only; >> + }; >> + partition@env { >> + label = "env"; >> + reg = <0x00100000 0x00010000>; >> + }; >> + partition@envred { >> + label = "envred"; >> + reg = <0x00110000 0x00010000>; >> + }; >> + partition@fman { >> + label = "fman-ucode"; >> + reg = <0x00120000 0x00010000>; >> + read-only; >> + }; >> + }; > > I realize it's common practice, but it would be good to get away from > putting partition layouts in the dts file. Alternatives include using > mtdparts on the command line, or having U-Boot put the partition info > into the dtb based on the mtdparts environment variable (there is > existing code for this). I agree that u-boot also has to know about the addresses because it also accesses these partitions. But I think it is clearer to have this in the device tree: I try to keep the kernel command line small and I don't like having u-boot "fixing" the dtb at runtime. > >> + zl30343@1 { >> + compatible = "gen,spidev"; > > Node names are supposed to be generic. Compatibles are supposed to be > specific. That's a very specific device for which we only have a userspace driver and for which we must use the generic kernel spidev driver. That's why the node name is so specific and the compatible field very generic. > >> + lbc: localbus@ffe124000 { >> + reg = <0xf 0xfe124000 0 0x1000>; >> + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ >> + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ >> + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ >> + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ >> + >> + nand@0,0 { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + compatible = "fsl,elbc-fcm-nand"; >> + reg = <0 0 0x40000>; >> + >> + partition@0 { >> + label = "ubi0"; >> + reg = <0x0 0x8000000>; >> + }; >> + }; >> + }; > > No nodes for those other chipselects? Well, there are nodes, but they are internally developed FPGAs and the drivers are not mainlined that's why I removed the nodes. > >> diff --git a/arch/powerpc/configs/85xx/kmp204x_defconfig b/arch/powerpc/configs/85xx/kmp204x_defconfig >> new file mode 100644 >> index 0000000..3bbf4fa >> --- /dev/null >> +++ b/arch/powerpc/configs/85xx/kmp204x_defconfig > > Why does this board need its own defconfig? Apart from the different drivers and FS that we use (or don't use) on the system, the most notable differences are: - lowmem must be set to a bigger size so that we can ioremap the the total memory requested for all of our PCIe devices - CGROUPS is enabled because that's a mandatory feature for our systems - NAND_ECC_BCH is enabled because it is used on all of our NAND devices > >> diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c >> index fbd871e..8e84e1c 100644 >> --- a/arch/powerpc/platforms/85xx/corenet_generic.c >> +++ b/arch/powerpc/platforms/85xx/corenet_generic.c >> @@ -122,6 +122,7 @@ static const char * const hv_boards[] __initconst = { >> NULL >> }; >> >> +#ifdef CONFIG_CORENET_GENERIC > > corenet_generic.c without CONFIG_CORENET_GENERIC? It would be possible with the CONFIG_KMP204X introduced with the patch. But this is linked with the below discussion. > >> /* >> * Called very early, device-tree isn't unflattened >> */ >> @@ -180,3 +181,54 @@ machine_arch_initcall(corenet_generic, corenet_gen_publish_devices); >> #ifdef CONFIG_SWIOTLB >> machine_arch_initcall(corenet_generic, swiotlb_setup_bus_notifier); >> #endif >> +#endif >> + >> +#ifdef CONFIG_KMP204X >> +/* >> + * Called very early, device-tree isn't unflattened >> + */ >> +static int __init kmp204x_generic_probe(void) >> +{ >> + unsigned long root = of_get_flat_dt_root(); >> + >> + return of_flat_dt_is_compatible(root, "keymile,kmp204x"); >> +} >> + >> + >> +/* >> + * Setup the architecture >> + */ >> +void __init kmp204x_gen_setup_arch(void) >> +{ >> + mpc85xx_smp_init(); >> + >> + swiotlb_detect_4g(); >> + >> + pr_info("%s platform from Keymile\n", ppc_md.name); >> +} >> + >> +define_machine(kmp204x) { >> + .name = "kmp204x", >> + .probe = kmp204x_generic_probe, >> + .setup_arch = kmp204x_gen_setup_arch, >> + .init_IRQ = corenet_gen_pic_init, >> +#ifdef CONFIG_PCI >> + .pcibios_fixup_bus = fsl_pcibios_fixup_bus, >> +#endif >> + .get_irq = mpic_get_coreint_irq, >> + .restart = fsl_rstcr_restart, >> + .calibrate_decr = generic_calibrate_decr, >> + .progress = udbg_progress, >> +#ifdef CONFIG_PPC64 >> + .power_save = book3e_idle, >> +#else >> + .power_save = e500_idle, >> +#endif >> +}; >> + >> +machine_arch_initcall(kmp204x, corenet_gen_publish_devices); >> + >> +#ifdef CONFIG_SWIOTLB >> +machine_arch_initcall(kmp204x, swiotlb_setup_bus_notifier); >> +#endif >> +#endif > > The whole point of corenet_generic.c is to avoid duplicating all of this > stuff. > > Can't you just use corenet_generic as-is other than adding the > compatible to boards[]? If not, explain why and put it in a different > file. > That's a valid point and I have to admit I have hesitated about that. I have mostly based my work on the FSL SDK where every single board has a "dedicated" file. I agree that I do nothing different than the corenet_generic does and adding my platform to the boards[] would be the same and you are right, I should use that and avoid code duplication. The only thing that would "bother" me is thus the pr_info print from *_gen_setup_arch(), it would be nice if somehow we could differentiate it or at least make it more generic since the kmp204x boards are not strictly boards from Freescale. Best regards Valentin
On Fri, 2014-01-17 at 13:51 +0100, Valentin Longchamp wrote: > Hi Scott, > > Thanks for you feedback. > > On 01/17/2014 12:35 AM, Scott Wood wrote: > > On Thu, 2014-01-16 at 14:38 +0100, Valentin Longchamp wrote: > >> This patch introduces the support for Keymile's kmp204x reference > >> design. This design is based on Freescale's P2040/P2041 SoC. > >> > >> The peripherals used by this design are: > >> - SPI NOR Flash as bootloader medium > >> - NAND Flash with a ubi partition > >> - 2 PCIe busses (hosts 1 and 3) > >> - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) > >> - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt > >> FPGA > >> - 2 HW I2C busses > >> - last but not least, the mandatory serial port > >> > >> The patch also adds a defconfig file for this reference design and a DTS > >> file for the kmcoge4 board which is the first one based on this > >> reference design. > >> > >> To try to avoid code duplication, the support was added directly to the > >> corenet_generic.c file. > >> > >> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > >> --- > >> arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ > >> arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ > >> arch/powerpc/platforms/85xx/Kconfig | 14 ++ > >> arch/powerpc/platforms/85xx/Makefile | 1 + > >> arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ > >> 5 files changed, 463 insertions(+) > >> create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts > >> create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig > >> > >> diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts > >> new file mode 100644 > >> index 0000000..c10df6d > >> --- /dev/null > >> +++ b/arch/powerpc/boot/dts/kmcoge4.dts > >> @@ -0,0 +1,165 @@ > >> +/* > >> + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS > >> + * > >> + * (C) Copyright 2014 > >> + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com > >> + * > >> + * Copyright 2011 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. > >> + */ > >> + > >> +/include/ "fsl/p2041si-pre.dtsi" > >> + > >> +/ { > >> + model = "keymile,kmcoge4"; > >> + compatible = "keymile,kmp204x"; > > > > Don't put wildcards in compatible. > > Well it's a wildcard in the sense that we support both the p2040 and the p2041, > but it's also the name of the plaftorm, similarly to names like '85xx' or 'tqm85xx'. Names like 85xx are not allowed in device trees. With "p204x", what would happen if a p2042 were introduced, that were not compatible? Why isn't the compatible "keymile,kmcoge4", like the model? > > I realize it's common practice, but it would be good to get away from > > putting partition layouts in the dts file. Alternatives include using > > mtdparts on the command line, or having U-Boot put the partition info > > into the dtb based on the mtdparts environment variable (there is > > existing code for this). > > I agree that u-boot also has to know about the addresses because it also > accesses these partitions. > > But I think it is clearer to have this in the device tree: I try to keep the > kernel command line small and I don't like having u-boot "fixing" the dtb at > runtime. The problem is that the dts source is often far removed from the actual programming of flash, and the partitioning can vary based on use case, or change for other reasons (e.g. there have been requests to change existing partition layouts to accommodate growth in U-Boot size). Ideally it wouldn't be in the device tree at all, but having U-Boot fix it up based on an environment variable is better than statically defining it in a file in the Linux tree. > >> + zl30343@1 { > >> + compatible = "gen,spidev"; > > > > Node names are supposed to be generic. Compatibles are supposed to be > > specific. > > That's a very specific device for which we only have a userspace driver and for > which we must use the generic kernel spidev driver. The device tree describes the hardware, not what driver you want to use. Plus, I don't see any driver that matches "gen,spidev" nor any binding for it, and "gen" doesn't make sense as a vendor prefix. The only instance of that string I can find in the Linux tree is in mgcoge.dts. > > That's why the node name is > > so specific and the compatible field very generic. Userspace can't search for a node by compatible? > >> + lbc: localbus@ffe124000 { > >> + reg = <0xf 0xfe124000 0 0x1000>; > >> + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ > >> + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ > >> + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ > >> + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ > >> + > >> + nand@0,0 { > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + compatible = "fsl,elbc-fcm-nand"; > >> + reg = <0 0 0x40000>; > >> + > >> + partition@0 { > >> + label = "ubi0"; > >> + reg = <0x0 0x8000000>; > >> + }; > >> + }; > >> + }; > > > > No nodes for those other chipselects? > > Well, there are nodes, but they are internally developed FPGAs and the drivers > are not mainlined that's why I removed the nodes. The device tree describes the hardware, not what drivers are currently mainlined in Linux. > >> diff --git a/arch/powerpc/configs/85xx/kmp204x_defconfig b/arch/powerpc/configs/85xx/kmp204x_defconfig > >> new file mode 100644 > >> index 0000000..3bbf4fa > >> --- /dev/null > >> +++ b/arch/powerpc/configs/85xx/kmp204x_defconfig > > > > Why does this board need its own defconfig? > > Apart from the different drivers and FS that we use (or don't use) on the > system, That is not generally a good reason for a separate defconfig. Just enable the drivers you need in the main defconfig, and don't worry about the drivers you don't need. You may want a smaller kernel for actual shipping products (though the changelog said this is a reference board...), but in mainline we want a small number of defconfigs that cover as many boards as possible (or at least, a reasonably small number and not one per board). > the most notable differences are: > - lowmem must be set to a bigger size so that we can ioremap the the total > memory requested for all of our PCIe devices > - CGROUPS is enabled because that's a mandatory feature for our systems > - NAND_ECC_BCH is enabled because it is used on all of our NAND devices I don't think there would be a problem adding CGROUPS or NAND_ECC_BCH to corenet32_smp_defconfig (though CGROUPS seems more like a use-case configuration than something to do with the board itself). The lowmem adjustment is probably a good reason, though I wish things like that could be specified as a defconfig that #includes corenet32_smp_defconfig and then just makes a couple changes. > > The whole point of corenet_generic.c is to avoid duplicating all of this > > stuff. > > > > Can't you just use corenet_generic as-is other than adding the > > compatible to boards[]? If not, explain why and put it in a different > > file. > > > > That's a valid point and I have to admit I have hesitated about that. I have > mostly based my work on the FSL SDK where every single board has a "dedicated" file. > > I agree that I do nothing different than the corenet_generic does and adding my > platform to the boards[] would be the same and you are right, I should use that > and avoid code duplication. > > The only thing that would "bother" me is thus the pr_info print from > *_gen_setup_arch(), it would be nice if somehow we could differentiate it or at > least make it more generic since the kmp204x boards are not strictly boards from > Freescale. Just remove the "from Freescale Semiconductor" part of the string. -Scott
On 01/17/2014 10:48 PM, Scott Wood wrote: > On Fri, 2014-01-17 at 13:51 +0100, Valentin Longchamp wrote: >> Hi Scott, >> >> Thanks for you feedback. >> >> On 01/17/2014 12:35 AM, Scott Wood wrote: >>> On Thu, 2014-01-16 at 14:38 +0100, Valentin Longchamp wrote: >>>> This patch introduces the support for Keymile's kmp204x reference >>>> design. This design is based on Freescale's P2040/P2041 SoC. >>>> >>>> The peripherals used by this design are: >>>> - SPI NOR Flash as bootloader medium >>>> - NAND Flash with a ubi partition >>>> - 2 PCIe busses (hosts 1 and 3) >>>> - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) >>>> - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt >>>> FPGA >>>> - 2 HW I2C busses >>>> - last but not least, the mandatory serial port >>>> >>>> The patch also adds a defconfig file for this reference design and a DTS >>>> file for the kmcoge4 board which is the first one based on this >>>> reference design. >>>> >>>> To try to avoid code duplication, the support was added directly to the >>>> corenet_generic.c file. >>>> >>>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> >>>> --- >>>> arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ >>>> arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ >>>> arch/powerpc/platforms/85xx/Kconfig | 14 ++ >>>> arch/powerpc/platforms/85xx/Makefile | 1 + >>>> arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ >>>> 5 files changed, 463 insertions(+) >>>> create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts >>>> create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig >>>> >>>> diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts >>>> new file mode 100644 >>>> index 0000000..c10df6d >>>> --- /dev/null >>>> +++ b/arch/powerpc/boot/dts/kmcoge4.dts >>>> @@ -0,0 +1,165 @@ >>>> +/* >>>> + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS >>>> + * >>>> + * (C) Copyright 2014 >>>> + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com >>>> + * >>>> + * Copyright 2011 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. >>>> + */ >>>> + >>>> +/include/ "fsl/p2041si-pre.dtsi" >>>> + >>>> +/ { >>>> + model = "keymile,kmcoge4"; >>>> + compatible = "keymile,kmp204x"; >>> >>> Don't put wildcards in compatible. >> >> Well it's a wildcard in the sense that we support both the p2040 and the p2041, >> but it's also the name of the plaftorm, similarly to names like '85xx' or 'tqm85xx'. > > Names like 85xx are not allowed in device trees. > > With "p204x", what would happen if a p2042 were introduced, that were > not compatible? What would you suggest as a generic name for the architecture that supports both ? > > Why isn't the compatible "keymile,kmcoge4", like the model? Because kmcoge4 is the board that is based on the kmp204x architecture/design. We expect other boards (kmcoge7 for instance) based on the same kmp204x design. You would prefer that I have the model and compatible stricly the same and add any future board into the compatible boards[] from corenet_generic ? If possible I would like to be able to see the boards that are based on a similar design, that's what I wanted to achieve with this kmp204x name. > >>> I realize it's common practice, but it would be good to get away from >>> putting partition layouts in the dts file. Alternatives include using >>> mtdparts on the command line, or having U-Boot put the partition info >>> into the dtb based on the mtdparts environment variable (there is >>> existing code for this). >> >> I agree that u-boot also has to know about the addresses because it also >> accesses these partitions. >> >> But I think it is clearer to have this in the device tree: I try to keep the >> kernel command line small and I don't like having u-boot "fixing" the dtb at >> runtime. > > The problem is that the dts source is often far removed from the actual > programming of flash, and the partitioning can vary based on use case, > or change for other reasons (e.g. there have been requests to change > existing partition layouts to accommodate growth in U-Boot size). > > Ideally it wouldn't be in the device tree at all, but having U-Boot fix > it up based on an environment variable is better than statically > defining it in a file in the Linux tree. > >>>> + zl30343@1 { >>>> + compatible = "gen,spidev"; >>> >>> Node names are supposed to be generic. Compatibles are supposed to be >>> specific. >> >> That's a very specific device for which we only have a userspace driver and for >> which we must use the generic kernel spidev driver. > > The device tree describes the hardware, not what driver you want to use. > > Plus, I don't see any driver that matches "gen,spidev" nor any binding > for it, and "gen" doesn't make sense as a vendor prefix. The only > instance of that string I can find in the Linux tree is in mgcoge.dts. Well it comes from mgcoge and that's why I have used this It's for usage with the spidev driver (driver/spi/spidev.c). I agree that the gen brings nothing. Would spidev@1 { compatible = "spidev"; make more sense ? > >>> That's why the node name is >>> so specific and the compatible field very generic. > > Userspace can't search for a node by compatible? > >>>> + lbc: localbus@ffe124000 { >>>> + reg = <0xf 0xfe124000 0 0x1000>; >>>> + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ >>>> + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ >>>> + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ >>>> + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ >>>> + >>>> + nand@0,0 { >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + compatible = "fsl,elbc-fcm-nand"; >>>> + reg = <0 0 0x40000>; >>>> + >>>> + partition@0 { >>>> + label = "ubi0"; >>>> + reg = <0x0 0x8000000>; >>>> + }; >>>> + }; >>>> + }; >>> >>> No nodes for those other chipselects? >> >> Well, there are nodes, but they are internally developed FPGAs and the drivers >> are not mainlined that's why I removed the nodes. > > The device tree describes the hardware, not what drivers are currently > mainlined in Linux. What do you want me to do: add the nodes for which there are no bindings ? I did this similarly to the situation with the FSL .dtsi that currently in mainline do not include the DPAA/QMAN/BMAN nodes. > >>>> diff --git a/arch/powerpc/configs/85xx/kmp204x_defconfig b/arch/powerpc/configs/85xx/kmp204x_defconfig >>>> new file mode 100644 >>>> index 0000000..3bbf4fa >>>> --- /dev/null >>>> +++ b/arch/powerpc/configs/85xx/kmp204x_defconfig >>> >>> Why does this board need its own defconfig? >> >> Apart from the different drivers and FS that we use (or don't use) on the >> system, > > That is not generally a good reason for a separate defconfig. Just > enable the drivers you need in the main defconfig, and don't worry about > the drivers you don't need. You may want a smaller kernel for actual > shipping products (though the changelog said this is a reference > board...), but in mainline we want a small number of defconfigs that > cover as many boards as possible (or at least, a reasonably small number > and not one per board). It's a reference design meaning that then all the further boards based on the kmp204x design would reuse that defconfig. But I understand that you want to avoid to multiply the number of defconfigs. > >> the most notable differences are: >> - lowmem must be set to a bigger size so that we can ioremap the the total >> memory requested for all of our PCIe devices >> - CGROUPS is enabled because that's a mandatory feature for our systems >> - NAND_ECC_BCH is enabled because it is used on all of our NAND devices > > I don't think there would be a problem adding CGROUPS or NAND_ECC_BCH to > corenet32_smp_defconfig (though CGROUPS seems more like a use-case > configuration than something to do with the board itself). The lowmem > adjustment is probably a good reason, though I wish things like that > could be specified as a defconfig that #includes corenet32_smp_defconfig > and then just makes a couple changes. > Yes that would be a nice feature to have: even for me, I would love to be able to rely on corenet32_smp_defconfig, include it and just add my changes. >>> The whole point of corenet_generic.c is to avoid duplicating all of this >>> stuff. >>> >>> Can't you just use corenet_generic as-is other than adding the >>> compatible to boards[]? If not, explain why and put it in a different >>> file. >>> >> >> That's a valid point and I have to admit I have hesitated about that. I have >> mostly based my work on the FSL SDK where every single board has a "dedicated" file. >> >> I agree that I do nothing different than the corenet_generic does and adding my >> platform to the boards[] would be the same and you are right, I should use that >> and avoid code duplication. >> >> The only thing that would "bother" me is thus the pr_info print from >> *_gen_setup_arch(), it would be nice if somehow we could differentiate it or at >> least make it more generic since the kmp204x boards are not strictly boards from >> Freescale. > > Just remove the "from Freescale Semiconductor" part of the string. > OK.
On Mon, 2014-01-20 at 17:38 +0100, Valentin Longchamp wrote: > On 01/17/2014 10:48 PM, Scott Wood wrote: > > On Fri, 2014-01-17 at 13:51 +0100, Valentin Longchamp wrote: > >> Hi Scott, > >> > >> Thanks for you feedback. > >> > >> On 01/17/2014 12:35 AM, Scott Wood wrote: > >>> On Thu, 2014-01-16 at 14:38 +0100, Valentin Longchamp wrote: > >>>> This patch introduces the support for Keymile's kmp204x reference > >>>> design. This design is based on Freescale's P2040/P2041 SoC. > >>>> > >>>> The peripherals used by this design are: > >>>> - SPI NOR Flash as bootloader medium > >>>> - NAND Flash with a ubi partition > >>>> - 2 PCIe busses (hosts 1 and 3) > >>>> - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) > >>>> - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt > >>>> FPGA > >>>> - 2 HW I2C busses > >>>> - last but not least, the mandatory serial port > >>>> > >>>> The patch also adds a defconfig file for this reference design and a DTS > >>>> file for the kmcoge4 board which is the first one based on this > >>>> reference design. > >>>> > >>>> To try to avoid code duplication, the support was added directly to the > >>>> corenet_generic.c file. > >>>> > >>>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> > >>>> --- > >>>> arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ > >>>> arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ > >>>> arch/powerpc/platforms/85xx/Kconfig | 14 ++ > >>>> arch/powerpc/platforms/85xx/Makefile | 1 + > >>>> arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ > >>>> 5 files changed, 463 insertions(+) > >>>> create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts > >>>> create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig > >>>> > >>>> diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts > >>>> new file mode 100644 > >>>> index 0000000..c10df6d > >>>> --- /dev/null > >>>> +++ b/arch/powerpc/boot/dts/kmcoge4.dts > >>>> @@ -0,0 +1,165 @@ > >>>> +/* > >>>> + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS > >>>> + * > >>>> + * (C) Copyright 2014 > >>>> + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com > >>>> + * > >>>> + * Copyright 2011 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. > >>>> + */ > >>>> + > >>>> +/include/ "fsl/p2041si-pre.dtsi" > >>>> + > >>>> +/ { > >>>> + model = "keymile,kmcoge4"; > >>>> + compatible = "keymile,kmp204x"; > >>> > >>> Don't put wildcards in compatible. > >> > >> Well it's a wildcard in the sense that we support both the p2040 and the p2041, > >> but it's also the name of the plaftorm, similarly to names like '85xx' or 'tqm85xx'. > > > > Names like 85xx are not allowed in device trees. > > > > With "p204x", what would happen if a p2042 were introduced, that were > > not compatible? > > What would you suggest as a generic name for the architecture that supports both ? > > > > > Why isn't the compatible "keymile,kmcoge4", like the model? > > Because kmcoge4 is the board that is based on the kmp204x architecture/design. > We expect other boards (kmcoge7 for instance) based on the same kmp204x design. The top-level compatible isn't for the "architecture" or the "design". It's for the board. Surely there's something different about kmcoge7 versus kmcoge4 -- is it visible to software? > You would prefer that I have the model and compatible stricly the same and add > any future board into the compatible boards[] from corenet_generic ? That's how it's usually done. Or, at least provide the board architecture name as a secondary compatible after the board name. > If possible I would like to be able to see the boards that are based on a > similar design, that's what I wanted to achieve with this kmp204x name. Is "kmp204x" an official name of the architecture, rather than a generalization of "kmp2040" and "kmp2041"? If there were a p2042, and you made a board for it, is there any chance it would be called kmp204x even if it were very different from the p2040/p2041 board? > >>>> + zl30343@1 { > >>>> + compatible = "gen,spidev"; > >>> > >>> Node names are supposed to be generic. Compatibles are supposed to be > >>> specific. > >> > >> That's a very specific device for which we only have a userspace driver and for > >> which we must use the generic kernel spidev driver. > > > > The device tree describes the hardware, not what driver you want to use. > > > > Plus, I don't see any driver that matches "gen,spidev" nor any binding > > for it, and "gen" doesn't make sense as a vendor prefix. The only > > instance of that string I can find in the Linux tree is in mgcoge.dts. > > Well it comes from mgcoge and that's why I have used this > > It's for usage with the spidev driver (driver/spi/spidev.c). I agree that the > gen brings nothing. Would > > spidev@1 { > compatible = "spidev"; > > make more sense ? It doesn't address any of the other comments. > >>>> + lbc: localbus@ffe124000 { > >>>> + reg = <0xf 0xfe124000 0 0x1000>; > >>>> + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ > >>>> + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ > >>>> + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ > >>>> + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ > >>>> + > >>>> + nand@0,0 { > >>>> + #address-cells = <1>; > >>>> + #size-cells = <1>; > >>>> + compatible = "fsl,elbc-fcm-nand"; > >>>> + reg = <0 0 0x40000>; > >>>> + > >>>> + partition@0 { > >>>> + label = "ubi0"; > >>>> + reg = <0x0 0x8000000>; > >>>> + }; > >>>> + }; > >>>> + }; > >>> > >>> No nodes for those other chipselects? > >> > >> Well, there are nodes, but they are internally developed FPGAs and the drivers > >> are not mainlined that's why I removed the nodes. > > > > The device tree describes the hardware, not what drivers are currently > > mainlined in Linux. > > What do you want me to do: add the nodes for which there are no bindings ? No, ideally you'd add bindings and nodes. I'm not going to insist on it if bindings aren't ready, but please don't leave things out only because there's no driver. > I did this similarly to the situation with the FSL .dtsi that currently in > mainline do not include the DPAA/QMAN/BMAN nodes. What we've done with DPAA doesn't make a very good role model, unfortunately. -Scott
On 01/20/2014 11:37 PM, Scott Wood wrote: > On Mon, 2014-01-20 at 17:38 +0100, Valentin Longchamp wrote: >> On 01/17/2014 10:48 PM, Scott Wood wrote: >>> On Fri, 2014-01-17 at 13:51 +0100, Valentin Longchamp wrote: >>>> Hi Scott, >>>> >>>> Thanks for you feedback. >>>> >>>> On 01/17/2014 12:35 AM, Scott Wood wrote: >>>>> On Thu, 2014-01-16 at 14:38 +0100, Valentin Longchamp wrote: >>>>>> This patch introduces the support for Keymile's kmp204x reference >>>>>> design. This design is based on Freescale's P2040/P2041 SoC. >>>>>> >>>>>> The peripherals used by this design are: >>>>>> - SPI NOR Flash as bootloader medium >>>>>> - NAND Flash with a ubi partition >>>>>> - 2 PCIe busses (hosts 1 and 3) >>>>>> - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) >>>>>> - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt >>>>>> FPGA >>>>>> - 2 HW I2C busses >>>>>> - last but not least, the mandatory serial port >>>>>> >>>>>> The patch also adds a defconfig file for this reference design and a DTS >>>>>> file for the kmcoge4 board which is the first one based on this >>>>>> reference design. >>>>>> >>>>>> To try to avoid code duplication, the support was added directly to the >>>>>> corenet_generic.c file. >>>>>> >>>>>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> >>>>>> --- >>>>>> arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ >>>>>> arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ >>>>>> arch/powerpc/platforms/85xx/Kconfig | 14 ++ >>>>>> arch/powerpc/platforms/85xx/Makefile | 1 + >>>>>> arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ >>>>>> 5 files changed, 463 insertions(+) >>>>>> create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts >>>>>> create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig >>>>>> >>>>>> diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts >>>>>> new file mode 100644 >>>>>> index 0000000..c10df6d >>>>>> --- /dev/null >>>>>> +++ b/arch/powerpc/boot/dts/kmcoge4.dts >>>>>> @@ -0,0 +1,165 @@ >>>>>> +/* >>>>>> + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS >>>>>> + * >>>>>> + * (C) Copyright 2014 >>>>>> + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com >>>>>> + * >>>>>> + * Copyright 2011 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. >>>>>> + */ >>>>>> + >>>>>> +/include/ "fsl/p2041si-pre.dtsi" >>>>>> + >>>>>> +/ { >>>>>> + model = "keymile,kmcoge4"; >>>>>> + compatible = "keymile,kmp204x"; >>>>> >>>>> Don't put wildcards in compatible. >>>> >>>> Well it's a wildcard in the sense that we support both the p2040 and the p2041, >>>> but it's also the name of the plaftorm, similarly to names like '85xx' or 'tqm85xx'. >>> >>> Names like 85xx are not allowed in device trees. >>> >>> With "p204x", what would happen if a p2042 were introduced, that were >>> not compatible? >> >> What would you suggest as a generic name for the architecture that supports both ? >> >>> >>> Why isn't the compatible "keymile,kmcoge4", like the model? >> >> Because kmcoge4 is the board that is based on the kmp204x architecture/design. >> We expect other boards (kmcoge7 for instance) based on the same kmp204x design. > > The top-level compatible isn't for the "architecture" or the "design". > It's for the board. Surely there's something different about kmcoge7 > versus kmcoge4 -- is it visible to software? There should only be a few differences in the dts between the two boards. Reading the ePAPR my understanding was that compatible is the "programming model" and that's what I have named above design/architecture while model is the exact model of the device in this case the exact board name. > >> You would prefer that I have the model and compatible stricly the same and add >> any future board into the compatible boards[] from corenet_generic ? > > That's how it's usually done. Or, at least provide the board > architecture name as a secondary compatible after the board name. > >> If possible I would like to be able to see the boards that are based on a >> similar design, that's what I wanted to achieve with this kmp204x name. > > Is "kmp204x" an official name of the architecture, rather than a > generalization of "kmp2040" and "kmp2041"? If there were a p2042, and > you made a board for it, is there any chance it would be called kmp204x > even if it were very different from the p2040/p2041 board? It's the name we have picked up, but it's not official. We also use km83xx, km82xx and it was derived from that. If the hypothetical p2042 board was different it would then have another name. > >>>>>> + zl30343@1 { >>>>>> + compatible = "gen,spidev"; >>>>> >>>>> Node names are supposed to be generic. Compatibles are supposed to be >>>>> specific. >>>> >>>> That's a very specific device for which we only have a userspace driver and for >>>> which we must use the generic kernel spidev driver. >>> >>> The device tree describes the hardware, not what driver you want to use. >>> >>> Plus, I don't see any driver that matches "gen,spidev" nor any binding >>> for it, and "gen" doesn't make sense as a vendor prefix. The only >>> instance of that string I can find in the Linux tree is in mgcoge.dts. >> >> Well it comes from mgcoge and that's why I have used this >> >> It's for usage with the spidev driver (driver/spi/spidev.c). I agree that the >> gen brings nothing. Would >> >> spidev@1 { >> compatible = "spidev"; >> >> make more sense ? > > It doesn't address any of the other comments. Can you please explicitly tell me how I should build this node ? What other comments ? Must I be more generic with the name ? Something like : spi@1 { compatible = "zarlink,30343", "spidev"; > >>>>>> + lbc: localbus@ffe124000 { >>>>>> + reg = <0xf 0xfe124000 0 0x1000>; >>>>>> + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ >>>>>> + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ >>>>>> + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ >>>>>> + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ >>>>>> + >>>>>> + nand@0,0 { >>>>>> + #address-cells = <1>; >>>>>> + #size-cells = <1>; >>>>>> + compatible = "fsl,elbc-fcm-nand"; >>>>>> + reg = <0 0 0x40000>; >>>>>> + >>>>>> + partition@0 { >>>>>> + label = "ubi0"; >>>>>> + reg = <0x0 0x8000000>; >>>>>> + }; >>>>>> + }; >>>>>> + }; >>>>> >>>>> No nodes for those other chipselects? >>>> >>>> Well, there are nodes, but they are internally developed FPGAs and the drivers >>>> are not mainlined that's why I removed the nodes. >>> >>> The device tree describes the hardware, not what drivers are currently >>> mainlined in Linux. >> >> What do you want me to do: add the nodes for which there are no bindings ? > > No, ideally you'd add bindings and nodes. I'm not going to insist on it > if bindings aren't ready, but please don't leave things out only because > there's no driver. Ideally we would add the bindings yes. But in the real world it's impossible for a company like us with limited resources to port the drivers of our FPGAs and their bindings to mainline. I will see what I can extract from the node and put them back. Valentin
On Tue, 2014-01-21 at 17:34 +0100, Valentin Longchamp wrote: > On 01/20/2014 11:37 PM, Scott Wood wrote: > > On Mon, 2014-01-20 at 17:38 +0100, Valentin Longchamp wrote: > >> On 01/17/2014 10:48 PM, Scott Wood wrote: > >>> Why isn't the compatible "keymile,kmcoge4", like the model? > >> > >> Because kmcoge4 is the board that is based on the kmp204x architecture/design. > >> We expect other boards (kmcoge7 for instance) based on the same kmp204x design. > > > > The top-level compatible isn't for the "architecture" or the "design". > > It's for the board. Surely there's something different about kmcoge7 > > versus kmcoge4 -- is it visible to software? > > There should only be a few differences in the dts between the two boards. > > Reading the ePAPR my understanding was that compatible is the "programming > model" and that's what I have named above design/architecture while model is the > exact model of the device in this case the exact board name. In practice, model is more for human consumption (e.g. there may be many variants that all look identical to software). The "programming model" for an entire board includes everything on it. > >> You would prefer that I have the model and compatible stricly the same and add > >> any future board into the compatible boards[] from corenet_generic ? > > > > That's how it's usually done. Or, at least provide the board > > architecture name as a secondary compatible after the board name. > > > >> If possible I would like to be able to see the boards that are based on a > >> similar design, that's what I wanted to achieve with this kmp204x name. > > > > Is "kmp204x" an official name of the architecture, rather than a > > generalization of "kmp2040" and "kmp2041"? If there were a p2042, and > > you made a board for it, is there any chance it would be called kmp204x > > even if it were very different from the p2040/p2041 board? > > It's the name we have picked up, but it's not official. We also use km83xx, > km82xx and it was derived from that. > > If the hypothetical p2042 board was different it would then have another name. In that case, I don't object to it being listed in compatible, though the specific board name should come first. > >>> The device tree describes the hardware, not what driver you want to use. > >>> > >>> Plus, I don't see any driver that matches "gen,spidev" nor any binding > >>> for it, and "gen" doesn't make sense as a vendor prefix. The only > >>> instance of that string I can find in the Linux tree is in mgcoge.dts. > >> > >> Well it comes from mgcoge and that's why I have used this > >> > >> It's for usage with the spidev driver (driver/spi/spidev.c). I agree that the > >> gen brings nothing. Would > >> > >> spidev@1 { > >> compatible = "spidev"; > >> > >> make more sense ? > > > > It doesn't address any of the other comments. > > Can you please explicitly tell me how I should build this node ? What other > comments ? Must I be more generic with the name ? > > Something like : > > spi@1 { > compatible = "zarlink,30343", "spidev"; Remove "spidev". Any nodes under the SPI controller node will be SPI devices, right? So it doesn't add anything regarding hardware description. -Scott
On 01/21/2014 06:01 PM, Scott Wood wrote: > On Tue, 2014-01-21 at 17:34 +0100, Valentin Longchamp wrote: >> On 01/20/2014 11:37 PM, Scott Wood wrote: >>> On Mon, 2014-01-20 at 17:38 +0100, Valentin Longchamp wrote: >>>> On 01/17/2014 10:48 PM, Scott Wood wrote: >>>>> Why isn't the compatible "keymile,kmcoge4", like the model? >>>> >>>> Because kmcoge4 is the board that is based on the kmp204x architecture/design. >>>> We expect other boards (kmcoge7 for instance) based on the same kmp204x design. >>> >>> The top-level compatible isn't for the "architecture" or the "design". >>> It's for the board. Surely there's something different about kmcoge7 >>> versus kmcoge4 -- is it visible to software? >> >> There should only be a few differences in the dts between the two boards. >> >> Reading the ePAPR my understanding was that compatible is the "programming >> model" and that's what I have named above design/architecture while model is the >> exact model of the device in this case the exact board name. > > In practice, model is more for human consumption (e.g. there may be many > variants that all look identical to software). The "programming model" > for an entire board includes everything on it. > >>>> You would prefer that I have the model and compatible stricly the same and add >>>> any future board into the compatible boards[] from corenet_generic ? >>> >>> That's how it's usually done. Or, at least provide the board >>> architecture name as a secondary compatible after the board name. >>> >>>> If possible I would like to be able to see the boards that are based on a >>>> similar design, that's what I wanted to achieve with this kmp204x name. >>> >>> Is "kmp204x" an official name of the architecture, rather than a >>> generalization of "kmp2040" and "kmp2041"? If there were a p2042, and >>> you made a board for it, is there any chance it would be called kmp204x >>> even if it were very different from the p2040/p2041 board? >> >> It's the name we have picked up, but it's not official. We also use km83xx, >> km82xx and it was derived from that. >> >> If the hypothetical p2042 board was different it would then have another name. > > In that case, I don't object to it being listed in compatible, though > the specific board name should come first. OK then to sum up both points we would have: model = "keymile,kmcoge4"; compatible = "keymile,kmcoge4", "keymile,kmp204x"; And I would add "keymile,kmcoge4" into the boards[] table. > >>>>> The device tree describes the hardware, not what driver you want to use. >>>>> >>>>> Plus, I don't see any driver that matches "gen,spidev" nor any binding >>>>> for it, and "gen" doesn't make sense as a vendor prefix. The only >>>>> instance of that string I can find in the Linux tree is in mgcoge.dts. >>>> >>>> Well it comes from mgcoge and that's why I have used this >>>> >>>> It's for usage with the spidev driver (driver/spi/spidev.c). I agree that the >>>> gen brings nothing. Would >>>> >>>> spidev@1 { >>>> compatible = "spidev"; >>>> >>>> make more sense ? >>> >>> It doesn't address any of the other comments. >> >> Can you please explicitly tell me how I should build this node ? What other >> comments ? Must I be more generic with the name ? >> >> Something like : >> >> spi@1 { >> compatible = "zarlink,30343", "spidev"; > > Remove "spidev". Any nodes under the SPI controller node will be SPI > devices, right? So it doesn't add anything regarding hardware > description. > OK. Thank you for the feedback, I will then send a revised patch as soon as I have time. Valentin
On Wed, 2014-01-22 at 17:38 +0100, Valentin Longchamp wrote: > On 01/21/2014 06:01 PM, Scott Wood wrote: > > On Tue, 2014-01-21 at 17:34 +0100, Valentin Longchamp wrote: > >> Can you please explicitly tell me how I should build this node ? What other > >> comments ? Must I be more generic with the name ? > >> > >> Something like : > >> > >> spi@1 { > >> compatible = "zarlink,30343", "spidev"; > > > > Remove "spidev". Any nodes under the SPI controller node will be SPI > > devices, right? So it doesn't add anything regarding hardware > > description. > > > > OK. > > Thank you for the feedback, I will then send a revised patch as soon as I have time. Oh, and ideally the node name should describe the function of the device -- "spi" as a node name usually means a SPI controller. Maybe "ptp_clock@1"? Also, zarlink should be added to Documentation/devicetree/bindings/vendor-prefixes.txt -Scott
diff --git a/arch/powerpc/boot/dts/kmcoge4.dts b/arch/powerpc/boot/dts/kmcoge4.dts new file mode 100644 index 0000000..c10df6d --- /dev/null +++ b/arch/powerpc/boot/dts/kmcoge4.dts @@ -0,0 +1,165 @@ +/* + * Keymile kmcoge4 Device Tree Source, based on the P2041RDB DTS + * + * (C) Copyright 2014 + * Valentin Longchamp, Keymile AG, valentin.longchamp@keymile.com + * + * Copyright 2011 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. + */ + +/include/ "fsl/p2041si-pre.dtsi" + +/ { + model = "keymile,kmcoge4"; + compatible = "keymile,kmp204x"; + #address-cells = <2>; + #size-cells = <2>; + interrupt-parent = <&mpic>; + + memory { + device_type = "memory"; + }; + + dcsr: dcsr@f00000000 { + ranges = <0x00000000 0xf 0x00000000 0x01008000>; + }; + + soc: soc@ffe000000 { + ranges = <0x00000000 0xf 0xfe000000 0x1000000>; + reg = <0xf 0xfe000000 0 0x00001000>; + spi@110000 { + flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "spansion,s25fl256s1"; + reg = <0>; + spi-max-frequency = <20000000>; /* input clock */ + partition@u-boot { + label = "u-boot"; + reg = <0x00000000 0x00100000>; + read-only; + }; + partition@env { + label = "env"; + reg = <0x00100000 0x00010000>; + }; + partition@envred { + label = "envred"; + reg = <0x00110000 0x00010000>; + }; + partition@fman { + label = "fman-ucode"; + reg = <0x00120000 0x00010000>; + read-only; + }; + }; + + zl30343@1 { + compatible = "gen,spidev"; + reg = <1>; + spi-max-frequency = <8000000>; + }; + + flash@2 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "micron,m25p32"; + reg = <2>; + spi-max-frequency = <15000000>; + partition@fpga-config { + label = "fpga-config"; + reg = <0x00000000 0x00400000>; + }; + }; + }; + + i2c@119000 { + status = "disabled"; + }; + + i2c@119100 { + status = "disabled"; + }; + + usb0: usb@210000 { + status = "disabled"; + }; + + usb1: usb@211000 { + status = "disabled"; + }; + + sata@220000 { + status = "disabled"; + }; + + sata@221000 { + status = "disabled"; + }; + }; + + rio: rapidio@ffe0c0000 { + status = "disabled"; + }; + + lbc: localbus@ffe124000 { + reg = <0xf 0xfe124000 0 0x1000>; + ranges = <0 0 0xf 0xffa00000 0x00040000 /* LB 0 */ + 1 0 0xf 0xfb000000 0x00010000 /* LB 1 */ + 2 0 0xf 0xd0000000 0x10000000 /* LB 2 */ + 3 0 0xf 0xe0000000 0x10000000>; /* LB 3 */ + + nand@0,0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "fsl,elbc-fcm-nand"; + reg = <0 0 0x40000>; + + partition@0 { + label = "ubi0"; + reg = <0x0 0x8000000>; + }; + }; + }; + + pci0: pcie@ffe200000 { + reg = <0xf 0xfe200000 0 0x1000>; + ranges = <0x02000000 0 0xe0000000 0xc 0x00000000 0x0 0x20000000 + 0x01000000 0 0x00000000 0xf 0xf8000000 0x0 0x00010000>; + pcie@0 { + ranges = <0x02000000 0 0xe0000000 + 0x02000000 0 0xe0000000 + 0 0x20000000 + + 0x01000000 0 0x00000000 + 0x01000000 0 0x00000000 + 0 0x00010000>; + }; + }; + + pci1: pcie@ffe201000 { + status = "disabled"; + }; + + pci2: pcie@ffe202000 { + reg = <0xf 0xfe202000 0 0x1000>; + ranges = <0x02000000 0 0xe0000000 0xc 0x40000000 0 0x20000000 + 0x01000000 0 0x00000000 0xf 0xf8020000 0 0x00010000>; + pcie@0 { + ranges = <0x02000000 0 0xe0000000 + 0x02000000 0 0xe0000000 + 0 0x20000000 + + 0x01000000 0 0x00000000 + 0x01000000 0 0x00000000 + 0 0x00010000>; + }; + }; +}; + +/include/ "fsl/p2041si-post.dtsi" diff --git a/arch/powerpc/configs/85xx/kmp204x_defconfig b/arch/powerpc/configs/85xx/kmp204x_defconfig new file mode 100644 index 0000000..3bbf4fa --- /dev/null +++ b/arch/powerpc/configs/85xx/kmp204x_defconfig @@ -0,0 +1,231 @@ +CONFIG_PPC_85xx=y +CONFIG_SMP=y +CONFIG_NR_CPUS=8 +CONFIG_SYSVIPC=y +CONFIG_POSIX_MQUEUE=y +CONFIG_AUDIT=y +CONFIG_NO_HZ=y +CONFIG_HIGH_RES_TIMERS=y +CONFIG_BSD_PROCESS_ACCT=y +CONFIG_IKCONFIG=y +CONFIG_IKCONFIG_PROC=y +CONFIG_LOG_BUF_SHIFT=14 +CONFIG_CGROUPS=y +CONFIG_CGROUP_SCHED=y +CONFIG_RELAY=y +CONFIG_BLK_DEV_INITRD=y +CONFIG_KALLSYMS_ALL=y +CONFIG_EMBEDDED=y +CONFIG_PERF_EVENTS=y +CONFIG_SLAB=y +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y +CONFIG_MODULE_FORCE_UNLOAD=y +CONFIG_MODVERSIONS=y +# CONFIG_BLK_DEV_BSG is not set +CONFIG_PARTITION_ADVANCED=y +CONFIG_MAC_PARTITION=y +CONFIG_KMP204X=y +CONFIG_MPIC_MSGR=y +CONFIG_CPU_FREQ=y +CONFIG_CPU_FREQ_GOV_ONDEMAND=y +CONFIG_HIGHMEM=y +# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set +CONFIG_BINFMT_MISC=m +CONFIG_KEXEC=y +CONFIG_FORCE_MAX_ZONEORDER=13 +CONFIG_PCI=y +CONFIG_PCIEPORTBUS=y +# CONFIG_PCIEASPM is not set +CONFIG_PCI_MSI=y +CONFIG_ADVANCED_OPTIONS=y +CONFIG_LOWMEM_SIZE_BOOL=y +CONFIG_LOWMEM_SIZE=0x20000000 +CONFIG_NET=y +CONFIG_PACKET=y +CONFIG_UNIX=y +CONFIG_XFRM_USER=y +CONFIG_XFRM_SUB_POLICY=y +CONFIG_XFRM_STATISTICS=y +CONFIG_NET_KEY=y +CONFIG_NET_KEY_MIGRATE=y +CONFIG_INET=y +CONFIG_IP_MULTICAST=y +CONFIG_IP_ADVANCED_ROUTER=y +CONFIG_IP_MULTIPLE_TABLES=y +CONFIG_IP_ROUTE_MULTIPATH=y +CONFIG_IP_ROUTE_VERBOSE=y +CONFIG_IP_PNP=y +CONFIG_IP_PNP_DHCP=y +CONFIG_IP_PNP_BOOTP=y +CONFIG_IP_PNP_RARP=y +CONFIG_NET_IPIP=y +CONFIG_IP_MROUTE=y +CONFIG_IP_PIMSM_V1=y +CONFIG_IP_PIMSM_V2=y +CONFIG_INET_AH=y +CONFIG_INET_ESP=y +CONFIG_INET_IPCOMP=y +# CONFIG_INET_LRO is not set +CONFIG_IPV6=y +CONFIG_IP_SCTP=m +CONFIG_TIPC=y +CONFIG_NET_SCHED=y +CONFIG_NET_SCH_CBQ=y +CONFIG_NET_SCH_HTB=y +CONFIG_NET_SCH_HFSC=y +CONFIG_NET_SCH_PRIO=y +CONFIG_NET_SCH_MULTIQ=y +CONFIG_NET_SCH_RED=y +CONFIG_NET_SCH_SFQ=y +CONFIG_NET_SCH_TEQL=y +CONFIG_NET_SCH_TBF=y +CONFIG_NET_SCH_GRED=y +CONFIG_NET_CLS_BASIC=y +CONFIG_NET_CLS_TCINDEX=y +CONFIG_NET_CLS_U32=y +CONFIG_CLS_U32_PERF=y +CONFIG_CLS_U32_MARK=y +CONFIG_NET_CLS_FLOW=y +CONFIG_NET_CLS_CGROUP=y +CONFIG_UEVENT_HELPER_PATH="/sbin/mdev" +CONFIG_DEVTMPFS=y +CONFIG_MTD=y +CONFIG_MTD_CMDLINE_PARTS=y +CONFIG_MTD_BLOCK=y +CONFIG_MTD_CFI=y +CONFIG_MTD_CFI_AMDSTD=y +CONFIG_MTD_PHYSMAP_OF=y +CONFIG_MTD_M25P80=y +CONFIG_MTD_PHRAM=y +CONFIG_MTD_NAND=y +CONFIG_MTD_NAND_ECC_BCH=y +CONFIG_MTD_NAND_FSL_ELBC=y +CONFIG_MTD_UBI=y +CONFIG_MTD_UBI_GLUEBI=y +CONFIG_PROC_DEVICETREE=y +CONFIG_BLK_DEV_LOOP=y +CONFIG_BLK_DEV_RAM=y +CONFIG_BLK_DEV_RAM_COUNT=2 +CONFIG_BLK_DEV_RAM_SIZE=2048 +CONFIG_EEPROM_AT24=y +CONFIG_SCSI=y +CONFIG_BLK_DEV_SD=y +CONFIG_CHR_DEV_ST=y +CONFIG_BLK_DEV_SR=y +CONFIG_CHR_DEV_SG=y +CONFIG_SCSI_MULTI_LUN=y +CONFIG_SCSI_LOGGING=y +CONFIG_SCSI_SYM53C8XX_2=y +CONFIG_NETDEVICES=y +# CONFIG_NET_VENDOR_3COM is not set +# CONFIG_NET_VENDOR_ADAPTEC is not set +# CONFIG_NET_VENDOR_ALTEON is not set +# CONFIG_NET_VENDOR_AMD is not set +# CONFIG_NET_VENDOR_ATHEROS is not set +# CONFIG_NET_CADENCE is not set +# CONFIG_NET_VENDOR_BROADCOM is not set +# CONFIG_NET_VENDOR_BROCADE is not set +# CONFIG_NET_VENDOR_CHELSIO is not set +# CONFIG_NET_VENDOR_CISCO is not set +# CONFIG_NET_VENDOR_DEC is not set +# CONFIG_NET_VENDOR_DLINK is not set +# CONFIG_NET_VENDOR_EMULEX is not set +# CONFIG_NET_VENDOR_EXAR is not set +CONFIG_FSL_PQ_MDIO=y +CONFIG_FSL_XGMAC_MDIO=y +# CONFIG_NET_VENDOR_HP is not set +# CONFIG_NET_VENDOR_INTEL is not set +# CONFIG_NET_VENDOR_MARVELL is not set +# CONFIG_NET_VENDOR_MELLANOX is not set +# CONFIG_NET_VENDOR_MICREL is not set +# CONFIG_NET_VENDOR_MICROCHIP is not set +# CONFIG_NET_VENDOR_MYRI is not set +# CONFIG_NET_VENDOR_NATSEMI is not set +# CONFIG_NET_VENDOR_NVIDIA is not set +# CONFIG_NET_VENDOR_OKI is not set +# CONFIG_NET_PACKET_ENGINE is not set +# CONFIG_NET_VENDOR_QLOGIC is not set +# CONFIG_NET_VENDOR_REALTEK is not set +# CONFIG_NET_VENDOR_RDC is not set +# CONFIG_NET_VENDOR_SEEQ is not set +# CONFIG_NET_VENDOR_SILAN is not set +# CONFIG_NET_VENDOR_SIS is not set +# CONFIG_NET_VENDOR_SMSC is not set +# CONFIG_NET_VENDOR_STMICRO is not set +# CONFIG_NET_VENDOR_SUN is not set +# CONFIG_NET_VENDOR_TEHUTI is not set +# CONFIG_NET_VENDOR_TI is not set +# CONFIG_NET_VENDOR_VIA is not set +# CONFIG_NET_VENDOR_WIZNET is not set +# CONFIG_NET_VENDOR_XILINX is not set +CONFIG_MARVELL_PHY=y +CONFIG_VITESSE_PHY=y +CONFIG_FIXED_PHY=y +# CONFIG_WLAN is not set +# CONFIG_INPUT_MOUSEDEV is not set +# CONFIG_INPUT_KEYBOARD is not set +# CONFIG_INPUT_MOUSE is not set +CONFIG_SERIO_LIBPS2=y +# CONFIG_LEGACY_PTYS is not set +CONFIG_PPC_EPAPR_HV_BYTECHAN=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_SERIAL_8250_MANY_PORTS=y +CONFIG_SERIAL_8250_DETECT_IRQ=y +CONFIG_SERIAL_8250_RSA=y +CONFIG_NVRAM=y +CONFIG_I2C=y +CONFIG_I2C_CHARDEV=y +CONFIG_I2C_MUX=y +CONFIG_I2C_MUX_PCA954x=y +CONFIG_I2C_MPC=y +CONFIG_SPI=y +CONFIG_SPI_GPIO=y +CONFIG_SPI_FSL_SPI=y +CONFIG_SPI_FSL_ESPI=y +CONFIG_SPI_SPIDEV=m +CONFIG_PTP_1588_CLOCK=y +# CONFIG_HWMON is not set +CONFIG_VIDEO_OUTPUT_CONTROL=y +# CONFIG_USB_SUPPORT is not set +CONFIG_EDAC=y +CONFIG_EDAC_MM_EDAC=y +CONFIG_EDAC_MPC85XX=y +CONFIG_RTC_CLASS=y +CONFIG_RTC_DRV_DS3232=y +CONFIG_RTC_DRV_CMOS=y +CONFIG_UIO=y +CONFIG_STAGING=y +# CONFIG_NET_VENDOR_SILICOM is not set +CONFIG_FSL_PAMU=y +CONFIG_EXT2_FS=y +CONFIG_NTFS_FS=y +CONFIG_PROC_KCORE=y +CONFIG_TMPFS=y +CONFIG_HUGETLBFS=y +CONFIG_JFFS2_FS=y +CONFIG_UBIFS_FS=y +CONFIG_CRAMFS=y +CONFIG_SQUASHFS=y +CONFIG_SQUASHFS_XZ=y +CONFIG_NFS_FS=y +CONFIG_NFS_V4=y +CONFIG_ROOT_NFS=y +CONFIG_NLS_ISO8859_1=y +CONFIG_NLS_UTF8=m +CONFIG_CRC_ITU_T=m +CONFIG_DEBUG_INFO=y +CONFIG_MAGIC_SYSRQ=y +CONFIG_DEBUG_SHIRQ=y +CONFIG_DETECT_HUNG_TASK=y +CONFIG_SCHEDSTATS=y +CONFIG_RCU_TRACE=y +CONFIG_UPROBE_EVENT=y +CONFIG_CRYPTO_NULL=y +CONFIG_CRYPTO_PCBC=m +CONFIG_CRYPTO_MD4=y +CONFIG_CRYPTO_SHA256=y +CONFIG_CRYPTO_SHA512=y +# CONFIG_CRYPTO_ANSI_CPRNG is not set +CONFIG_CRYPTO_DEV_FSL_CAAM=y diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig index 4d46349..78b849b 100644 --- a/arch/powerpc/platforms/85xx/Kconfig +++ b/arch/powerpc/platforms/85xx/Kconfig @@ -263,6 +263,20 @@ config CORENET_GENERIC The following boards are supported for both 32bit and 64bit kernel: P5020 DS and P5040 DS +config KMP204X + bool "Keymile kmp204x generic platform" + select DEFAULT_UIMAGE + select E500 + select PPC_E500MC + select PHYS_64BIT + select SWIOTLB + select ARCH_REQUIRE_GPIOLIB + select GPIO_MPC8XXX + select PPC_EPAPR_HV_PIC + help + This option enables support for the Keymile boards based on the + kmp204x platform. + endif # FSL_SOC_BOOKE config TQM85xx diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile index dd4c0b5..76042e1 100644 --- a/arch/powerpc/platforms/85xx/Makefile +++ b/arch/powerpc/platforms/85xx/Makefile @@ -29,3 +29,4 @@ obj-$(CONFIG_XES_MPC85xx) += xes_mpc85xx.o obj-$(CONFIG_GE_IMP3A) += ge_imp3a.o obj-$(CONFIG_PPC_QEMU_E500) += qemu_e500.o obj-$(CONFIG_SGY_CTS1000) += sgy_cts1000.o +obj-$(CONFIG_KMP204X) += corenet_generic.o diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c index fbd871e..8e84e1c 100644 --- a/arch/powerpc/platforms/85xx/corenet_generic.c +++ b/arch/powerpc/platforms/85xx/corenet_generic.c @@ -122,6 +122,7 @@ static const char * const hv_boards[] __initconst = { NULL }; +#ifdef CONFIG_CORENET_GENERIC /* * Called very early, device-tree isn't unflattened */ @@ -180,3 +181,54 @@ machine_arch_initcall(corenet_generic, corenet_gen_publish_devices); #ifdef CONFIG_SWIOTLB machine_arch_initcall(corenet_generic, swiotlb_setup_bus_notifier); #endif +#endif + +#ifdef CONFIG_KMP204X +/* + * Called very early, device-tree isn't unflattened + */ +static int __init kmp204x_generic_probe(void) +{ + unsigned long root = of_get_flat_dt_root(); + + return of_flat_dt_is_compatible(root, "keymile,kmp204x"); +} + + +/* + * Setup the architecture + */ +void __init kmp204x_gen_setup_arch(void) +{ + mpc85xx_smp_init(); + + swiotlb_detect_4g(); + + pr_info("%s platform from Keymile\n", ppc_md.name); +} + +define_machine(kmp204x) { + .name = "kmp204x", + .probe = kmp204x_generic_probe, + .setup_arch = kmp204x_gen_setup_arch, + .init_IRQ = corenet_gen_pic_init, +#ifdef CONFIG_PCI + .pcibios_fixup_bus = fsl_pcibios_fixup_bus, +#endif + .get_irq = mpic_get_coreint_irq, + .restart = fsl_rstcr_restart, + .calibrate_decr = generic_calibrate_decr, + .progress = udbg_progress, +#ifdef CONFIG_PPC64 + .power_save = book3e_idle, +#else + .power_save = e500_idle, +#endif +}; + +machine_arch_initcall(kmp204x, corenet_gen_publish_devices); + +#ifdef CONFIG_SWIOTLB +machine_arch_initcall(kmp204x, swiotlb_setup_bus_notifier); +#endif +#endif
This patch introduces the support for Keymile's kmp204x reference design. This design is based on Freescale's P2040/P2041 SoC. The peripherals used by this design are: - SPI NOR Flash as bootloader medium - NAND Flash with a ubi partition - 2 PCIe busses (hosts 1 and 3) - 3 FMAN Ethernet devices (FMAN1 DTSEC1/2/5) - 4 Local Bus windows, with one dedicated to the QRIO reset/power mgmt FPGA - 2 HW I2C busses - last but not least, the mandatory serial port The patch also adds a defconfig file for this reference design and a DTS file for the kmcoge4 board which is the first one based on this reference design. To try to avoid code duplication, the support was added directly to the corenet_generic.c file. Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com> --- arch/powerpc/boot/dts/kmcoge4.dts | 165 ++++++++++++++++++ arch/powerpc/configs/85xx/kmp204x_defconfig | 231 ++++++++++++++++++++++++++ arch/powerpc/platforms/85xx/Kconfig | 14 ++ arch/powerpc/platforms/85xx/Makefile | 1 + arch/powerpc/platforms/85xx/corenet_generic.c | 52 ++++++ 5 files changed, 463 insertions(+) create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig