[RFC PATCH 1/1] powerpc/embedded6xx: Add support for Motorola/Emerson MVME5100.
Scott Wood
scottwood at freescale.com
Fri Aug 9 11:35:20 EST 2013
On Thu, 2013-08-08 at 11:03 +1100, Stephen N Chivers wrote:
> Add support for the Motorola/Emerson MVME5100 Single Board Computer.
>
> The MVME5100 is a 6U form factor VME64 computer with:
>
> - A single MPC7410 or MPC750 CPU
> - A HAWK Processor Host Bridge (CPU to PCI) and
> MultiProcessor Interrupt Controller (MPIC)
> - Up to 500Mb of onboard memory
> - A M48T37 Real Time Clock (RTC) and Non-Volatile Memory chip
> - Two 16550 compatible UARTS
> - Two Intel E100 Fast Ethernets
> - Two PCI Mezzanine Card (PMC) Slots
> - PPCBug Firmware
>
> The HAWK PHB/MPIC is compatible with the MPC10x devices.
>
> There is no onboard disk support. This is usually provided by installing a
> PMC
> in first PMC slot.
>
> This patch revives the board support, it was present in early 2.6
> series kernels. The board support in those days was by Matt Porter of
> MontaVista Software.
>
> CSC Australia has around 31 of these boards in service. The kernel in use
> for the boards is based on 2.6.31. The boards are operated without disks
> from a file server.
>
> This patch is based on linux-3.11-rc4 and has been boot tested.
>
> Signed-off-by: Stephen Chivers <schivers at csc.com>
> ---
> arch/powerpc/boot/dts/mvme5100.dts | 195 ++
> arch/powerpc/boot/mvme5100.c | 28 +
> arch/powerpc/configs/mvme5100_defconfig | 2597
> +++++++++++++++++++++++++
> arch/powerpc/platforms/embedded6xx/mvme5100.c | 288 +++
> 4 files changed, 3108 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mvme5100.dts
> b/arch/powerpc/boot/dts/mvme5100.dts
> new file mode 100644
> index 0000000..17fdbc7
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/mvme5100.dts
> @@ -0,0 +1,195 @@
> +/*
> + * Device Tree Souce for Motorola/Emerson MVME5100.
> + *
> + * Copyright 2013 CSC Australia Pty. Ltd.
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without
> + * any warranty of any kind, whether express or implied.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> + model = "MVME5100";
> + compatible = "MVME5100";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + aliases {
> + serial0 = &serial0;
> + pci0 = &pci0;
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + PowerPC,7410 {
> + device_type = "cpu";
> + reg = <0x0>;
> + /* Following required by dtc but not used */
> + d-cache-line-size = <32>;
> + i-cache-line-size = <32>;
> + i-cache-size = <32768>;
> + d-cache-size = <32768>;
> + timebase-frequency = <25000000>;
> + clock-frequency = <500000000>;
> + bus-frequency = <100000000>;
What does "following" mean? certainly some of those are used, such as
timebase-frequency. In any case, it's not the device tree's job to
document which properties are used by Linux.
> + };
> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x0 0x20000000>;
> + };
> +
> + hawk at fef8 {
Unit address does not match reg.
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "soc";
Is this really an SoC? In any case, this use of device_type is
deprecated.
> + compatible = "mpc10x";
Don't use wildcards in compatible strings.
simple-bus may be applicable here (in addition to a specific
compatible).
> + store-gathering = <0>;
> + ranges = <0x0 0xfef80000 0x10000>;
> + reg = <0xfef80000 0x10000>;
Where is "store-gathering" documented?
> + serial0: serial at 8000 {
> + cell-index = <0>;
> + device_type = "serial";
> + compatible = "ns16550";
> + reg = <0x8000 0x80>;
> + reg-shift = <4>;
> + clock-frequency = <1843200>;
> + current-speed = <9600>;
> + interrupts = <1 1>; // IRQ1 Level Active Low.
> + interrupt-parent = <&mpic>;
> + };
> +
> + serial1: serial at 8200 {
> + cell-index = <1>;
> + device_type = "serial";
> + compatible = "ns16550";
> + reg = <0x8200 0x80>;
> + reg-shift = <4>;
> + clock-frequency = <1843200>;
> + current-speed = <9600>;
> + interrupts = <1 1>; // IRQ1 Level Active Low.
> + interrupt-parent = <&mpic>;
> + };
What specifically does cell-index mean here? Is it describing the
hardware or just the terminology used in documentation?
> + pci0: pci at feff0000 {
> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + device_type = "pci";
> + compatible = "mpc10x-pci";
> + reg = <0xfec00000 0x400000>;
> + 8259-interrupt-acknowledge = <0xfeff0030>;
Where is 8259-interrupt-acknowledge documented?
> + ranges = <0x1000000 0x0 0x0 0xfe000000 0x0 0x800000
> + 0x2000000 0x0 0x80000000 0x80000000 0x0
> 0x74000000>;
> + bus-range = <0 255>;
> + clock-frequency = <33333333>;
> + interrupt-parent = <&mpic>;
> + interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
> + interrupt-map = <
> +
> + /*
> + * This definition (IDSEL 11) duplicates the
> + * interrupts definition in the i8259
> + * interrupt controller below.
> + *
> + * It isn't essential to provide it and the
> + * board will run happily without it.
Should it be present then?
> + isa {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + #interrupt-cells = <2>;
> + device_type = "isa";
> + compatible = "isa";
> + ranges = <0x00000001 0 0x01000000 0 0x00000000
> 0x00001000>;
> + interrupt-parent = <&i8259>;
> +
> + i8259: interrupt-controller at 20 {
> + #interrupt-cells = <2>;
> + #address-cells = <0>;
> + interrupts = <0 2>;
> + device_type = "interrupt-controller";
> + compatible = "chrp,iic";
> + interrupt-controller;
> + reg = <1 0x00000020 0x00000002
> + 1 0x000000a0 0x00000002
> + 1 0x000004d0 0x00000002>;
> + clock-frequency = <0>;
Why does an i8259 have a clock-frequency?
> + built-in;
What does this mean?
> diff --git a/arch/powerpc/platforms/embedded6xx/mvme5100.c
> b/arch/powerpc/platforms/embedded6xx/mvme5100.c
> new file mode 100644
> index 0000000..cc9ed8a
> --- /dev/null
> +++ b/arch/powerpc/platforms/embedded6xx/mvme5100.c
> @@ -0,0 +1,288 @@
> +/*
> + * Board setup routines for the Motorola/Emerson MVME5100.
> + *
> + * Copyright 2013 CSC Australia Pty. Ltd.
> + *
> + * Based on earlier code by:
> + *
> + * Matt Porter, MontaVista Software Inc.
> + * Copyright 2001 MontaVista Software 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.
> + *
> + * Author: Stephen Chivers <schivers at csc.com>
> + *
> + */
> +
> +#include <linux/of_platform.h>
> +
> +#include <asm/i8259.h>
> +#include <asm/pci-bridge.h>
> +#include <asm/mpic.h>
> +#include <asm/prom.h>
> +#include <mm/mmu_decl.h>
> +#include <asm/udbg.h>
> +
> +
> +#define HAWK_MPIC_SIZE 0x00040000U
> +#define MVME5100_PCI_MEM_OFFSET 0x00000000
> +
> +/* Board register addresses. */
> +#define BOARD_STATUS_REG 0xfef88080
> +#define BOARD_MODFAIL_REG 0xfef88090
> +#define BOARD_MODRST_REG 0xfef880a0
> +#define BOARD_TBEN_REG 0xfef880c0
> +#define BOARD_SW_READ_REG 0xfef880e0
> +#define BOARD_GEO_ADDR_REG 0xfef880e8
> +#define BOARD_EXT_FEATURE1_REG 0xfef880f0
> +#define BOARD_EXT_FEATURE2_REG 0xfef88100
> +
> +static unsigned int pci_membase;
> +
> +
> +#ifdef DEBUG
> +#define DBG(fmt, args...) printk(KERN_ERR "%s: " fmt, __func__, ## args)
> +#else
> +#define DBG(fmt, args...)
> +#endif
Use pr_debug().
> +static void
> +mvme5100_8259_cascade(unsigned int irq, struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + unsigned int cascade_irq = i8259_irq();
> +
> +
Only one blank line.
> + if (cascade_irq != NO_IRQ)
> + generic_handle_irq(cascade_irq);
> +
> + chip->irq_eoi(&desc->irq_data);
> +}
> +
> +
Only one blank line. Likewise elsewhere.
> +static void __init
> +mvme5100_pic_init(void)
Don't put a newline before the function name.
> +{
> + struct mpic *mpic;
> + void __iomem *mpic_addr;
> + struct device_node *np;
> + struct device_node *cp = NULL;
> + unsigned int cirq;
> + unsigned long intack = 0;
> + const unsigned long *prop = NULL;
Do not use non-fixed-size-types to decode properties. In particular,
non-string properties are normally composed of 32-bit cells.
> + np = of_find_node_by_type(NULL, "open-pic");
> + if (!np) {
> + pr_err("Could not find open-pic node\n");
> + return;
> + }
> +
> +
> + pr_info("mvme5100_pic_init: pci_membase: %x\n", pci_membase);
> +
> + mpic_addr = ioremap(pci_membase + MVME5100_PCI_MEM_OFFSET,
> + HAWK_MPIC_SIZE);
> +
> + pr_info("mvme5100_pic_init: mapped mpic_addr: 0x%p\n", mpic_addr);
The only thing you use this ioremap for is to print out the address.
> + for_each_node_by_type(np, "interrupt-controller")
> + if (of_device_is_compatible(np, "chrp,iic")) {
> + cp = np;
> + break;
> + }
> +
Please use braces around multi-line loop bodies.
Why not just look for a chrp,iic node directly?
> + if ((np = of_find_compatible_node(NULL, "pci", "mpc10x-pci"))) {
Why insist on the device_type?
> +static int __init
> +mvme5100_add_bridge(struct device_node *dev)
> +{
> + const int *bus_range;
> + int len;
> + struct pci_controller *hose;
> + unsigned short devid;
> +
> +
> + pr_info("Adding PCI host bridge %s\n", dev->full_name);
> +
> + bus_range = of_get_property(dev, "bus-range", &len);
> +
> + if (bus_range == NULL || len < 2 * sizeof(int))
> + pr_warn("Can't get bus-range for %s, assuming bus 0\n",
> + dev->full_name);
Is this worth warning about?
> + if (devid != PCI_DEVICE_ID_MOTOROLA_HAWK) {
> + pr_err("HAWK PHB not present?\n");
> +
> + return 0;
> + }
> +
> + early_read_config_dword( hose, 0, 0, PCI_BASE_ADDRESS_1,
> &pci_membase);
Patch is linewrapped.
> +static void
> +mvme5100_restart(char *cmd)
> +{
> + volatile ulong i = 10000000;
> +
> +
> + local_irq_disable();
> + _nmask_and_or_msr(0, MSR_IP);
Does "mtmsr(mfmsr() | MSR_IP)" not work?
> + out_8((u_char *) BOARD_MODRST_REG, 0x01);
> +
> + while (i-- > 0);
Do not use a loop to implement a delay.
> +static void __init
> +mvme5100_set_bat(void)
> +{
> +
> +
> + mb();
> + mtspr(SPRN_DBAT1U, 0xf0001ffe);
> + mtspr(SPRN_DBAT1L, 0xf000002a);
> + mb();
> + setbat(1, 0xfe000000, 0xfe000000, 0x02000000, PAGE_KERNEL_NCG);
> +}
It is no longer allowed to squat on random virtual address space like
this. If you really need a BAT you'll have to allocate the virtual
address properly.
-Scott
More information about the Linuxppc-dev
mailing list