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.longch...@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.longch...@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
