Re: [PATCH 4/5] MIPS: xilfpga: Add mipsfpga platform code

From: James Hogan
Date: Thu Oct 15 2015 - 04:12:07 EST


On Wed, Oct 14, 2015 at 01:51:56PM +0100, Zubair Lutfullah Kakakhel wrote:
> The xilfpga platform will be DT only.
>
> Add required platform code. DT files have already been added separately

Please wrap to 72 characters (keeps it looking pretty in git-log
output), and maybe add a full stop.

>
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> ---
> arch/mips/Kbuild.platforms | 1 +
> arch/mips/Kconfig | 25 ++++++++++++++
> arch/mips/include/asm/mach-xilfpga/gpio.h | 19 +++++++++++
> arch/mips/include/asm/mach-xilfpga/irq.h | 18 ++++++++++
> arch/mips/xilfpga/Kconfig | 9 +++++
> arch/mips/xilfpga/Makefile | 7 ++++
> arch/mips/xilfpga/Platform | 3 ++
> arch/mips/xilfpga/init.c | 57 +++++++++++++++++++++++++++++++
> arch/mips/xilfpga/intc.c | 26 ++++++++++++++
> arch/mips/xilfpga/time.c | 41 ++++++++++++++++++++++
> 10 files changed, 206 insertions(+)
> create mode 100644 arch/mips/include/asm/mach-xilfpga/gpio.h
> create mode 100644 arch/mips/include/asm/mach-xilfpga/irq.h
> create mode 100644 arch/mips/xilfpga/Kconfig
> create mode 100644 arch/mips/xilfpga/Makefile
> create mode 100644 arch/mips/xilfpga/Platform
> create mode 100644 arch/mips/xilfpga/init.c
> create mode 100644 arch/mips/xilfpga/intc.c
> create mode 100644 arch/mips/xilfpga/time.c
>
> diff --git a/arch/mips/Kbuild.platforms b/arch/mips/Kbuild.platforms
> index a424e46..a96c81d 100644
> --- a/arch/mips/Kbuild.platforms
> +++ b/arch/mips/Kbuild.platforms
> @@ -33,6 +33,7 @@ platforms += sibyte
> platforms += sni
> platforms += txx9
> platforms += vr41xx
> +platforms += xilfpga
>
> # include the platform specific files
> include $(patsubst %, $(srctree)/arch/mips/%/Platform, $(platforms))
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index e3aa5b0..66c1a7e 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -401,6 +401,30 @@ config MACH_PISTACHIO
> help
> This enables support for the IMG Pistachio SoC platform.
>
> +config MACH_XILFPGA
> + bool "MIPSFPGA Xilinx based boards"
> + select ARCH_REQUIRE_GPIOLIB
> + select BOOT_ELF32
> + select BOOT_RAW
> + select BUILTIN_DTB
> + select CEVT_R4K
> + select COMMON_CLK
> + select CSRC_R4K
> + select IRQ_MIPS_CPU
> + select LIBFDT
> + select MIPS_CPU_SCACHE
> + select SERIAL_8250
> + select SERIAL_8250_CONSOLE

Is it really necessary to select SERIAL_8250[_CONSOLE]? Its something
that I think belongs in the defconfig rather than as a strict
requirement to enable the platform.

> + select SYS_HAS_EARLY_PRINTK
> + select SYS_HAS_CPU_MIPS32_R2
> + select SYS_SUPPORTS_32BIT_KERNEL
> + select SYS_SUPPORTS_LITTLE_ENDIAN
> + select SYS_SUPPORTS_ZBOOT_UART16550
> + select USE_OF
> + select USE_GENERIC_EARLY_PRINTK_8250
> + help
> + This enables support for the Xilinx

Is some description missing here?

> +
> config MIPS_MALTA
> bool "MIPS Malta board"
> select ARCH_MAY_HAVE_PC_FDC
> @@ -964,6 +988,7 @@ source "arch/mips/loongson32/Kconfig"
> source "arch/mips/loongson64/Kconfig"
> source "arch/mips/netlogic/Kconfig"
> source "arch/mips/paravirt/Kconfig"
> +source "arch/mips/xilfpga/Kconfig"
>
> endmenu
>
> diff --git a/arch/mips/include/asm/mach-xilfpga/gpio.h b/arch/mips/include/asm/mach-xilfpga/gpio.h
> new file mode 100644
> index 0000000..26778fc
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-xilfpga/gpio.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> + *
> + * 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.
> + */
> +
> +#ifndef __ASM_MACH_XILFPGA_GPIO_H
> +#define __ASM_MACH_XILFPGA_GPIO_H
> +
> +#include <asm-generic/gpio.h>
> +
> +#define gpio_get_value __gpio_get_value
> +#define gpio_set_value __gpio_set_value
> +
> +#endif /* __ASM_MACH_XILFPGA_GPIO_H */
> diff --git a/arch/mips/include/asm/mach-xilfpga/irq.h b/arch/mips/include/asm/mach-xilfpga/irq.h
> new file mode 100644
> index 0000000..0132a5b9
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-xilfpga/irq.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> + *
> + * 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.
> + */
> +
> +#ifndef __MIPS_ASM_MACH_XILFPGA_IRQ_H__
> +#define __MIPS_ASM_MACH_XILFPGA_IRQ_H__
> +
> +#define NR_IRQS 32
> +
> +#include_next <irq.h>
> +
> +#endif /* __MIPS_ASM_MACH_XILFPGA_IRQ_H__ */
> diff --git a/arch/mips/xilfpga/Kconfig b/arch/mips/xilfpga/Kconfig
> new file mode 100644
> index 0000000..42a030a
> --- /dev/null
> +++ b/arch/mips/xilfpga/Kconfig
> @@ -0,0 +1,9 @@
> +choice
> + prompt "Machine type"
> + depends on MACH_XILFPGA
> + default XILFPGA_NEXYS4DDR
> +
> +config XILFPGA_NEXYS4DDR
> + bool "Nexys4DDR by Digilent"
> +
> +endchoice
> diff --git a/arch/mips/xilfpga/Makefile b/arch/mips/xilfpga/Makefile
> new file mode 100644
> index 0000000..2385aa7
> --- /dev/null
> +++ b/arch/mips/xilfpga/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for the Xilfpga
> +#
> +
> +obj-y += init.o \
> + intc.o \
> + time.o

Can I suggest instead doing this:
obj-y += init.o
obj-y += intc.o
obj-y += time.o

> diff --git a/arch/mips/xilfpga/Platform b/arch/mips/xilfpga/Platform
> new file mode 100644
> index 0000000..ed375af
> --- /dev/null
> +++ b/arch/mips/xilfpga/Platform
> @@ -0,0 +1,3 @@
> +platform-$(CONFIG_MACH_XILFPGA) += xilfpga/
> +cflags-$(CONFIG_MACH_XILFPGA) += -I$(srctree)/arch/mips/include/asm/mach-xilfpga
> +load-$(CONFIG_MACH_XILFPGA) += 0xffffffff80100000
> diff --git a/arch/mips/xilfpga/init.c b/arch/mips/xilfpga/init.c
> new file mode 100644
> index 0000000..5348af8
> --- /dev/null
> +++ b/arch/mips/xilfpga/init.c
> @@ -0,0 +1,57 @@
> +/*
> + * Xilfpga platform setup
> + *
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/of_fdt.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/prom.h>
> +
> +#define XILFPGA_UART_BASE 0xb0401000
> +
> +const char *get_system_type(void)
> +{
> + return "MIPSFpga";
> +}
> +
> +void __init plat_mem_setup(void)
> +{
> + __dt_setup_arch(__dtb_start);
> + strlcpy(arcs_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> +}
> +
> +void __init prom_init(void)
> +{
> + setup_8250_early_printk_port(XILFPGA_UART_BASE, 2, 50000);
> +}
> +
> +void __init prom_free_prom_memory(void)
> +{
> +}
> +
> +void __init device_tree_init(void)
> +{
> + if (!initial_boot_params)
> + return;
> +
> + unflatten_and_copy_device_tree();
> +}
> +
> +static int __init plat_of_setup(void)
> +{
> + if (!of_have_populated_dt())
> + panic("Device tree not present");
> +
> + if (of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL))
> + panic("Failed to populate DT");
> +
> + return 0;
> +}
> +arch_initcall(plat_of_setup);
> diff --git a/arch/mips/xilfpga/intc.c b/arch/mips/xilfpga/intc.c
> new file mode 100644
> index 0000000..919eb97
> --- /dev/null
> +++ b/arch/mips/xilfpga/intc.c
> @@ -0,0 +1,26 @@
> +/*
> + * Xilfpga interrupt controller setup
> + *
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +
> +#include <asm/irq_cpu.h>
> +
> +static struct of_device_id of_irq_ids[] __initdata = {
> + { .compatible = "mti,cpu-interrupt-controller", .data = mips_cpu_irq_of_init },
> + {},
> +};
> +
> +void __init arch_init_irq(void)
> +{
> + of_irq_init(of_irq_ids);
> +}
> +

no need for extra newline at end of file.

> diff --git a/arch/mips/xilfpga/time.c b/arch/mips/xilfpga/time.c
> new file mode 100644
> index 0000000..3f2e39e
> --- /dev/null
> +++ b/arch/mips/xilfpga/time.c
> @@ -0,0 +1,41 @@
> +/*
> + * Xilfpga clocksource/timer setup
> + *
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>

You don't appear to be providing any clocks, so do you actually need
that include?

> +#include <linux/clocksource.h>
> +#include <linux/of.h>
> +
> +#include <asm/time.h>
> +
> +void __init plat_time_init(void)
> +{
> + struct device_node *np;
> + struct clk *clk = 0;

No need to initialise to 0 since you always set before use (and this is
C, so NULL is better than 0 anyway).

Cheers
James

> +
> + of_clk_init(NULL);
> + clocksource_of_init();
> +
> + np = of_get_cpu_node(0, NULL);
> + if (!np) {
> + pr_err("Failed to get CPU node\n");
> + return;
> + }
> +
> + clk = of_clk_get(np, 0);
> + if (IS_ERR(clk)) {
> + pr_err("Failed to get CPU clock: %ld\n", PTR_ERR(clk));
> + return;
> + }
> +
> + mips_hpt_frequency = clk_get_rate(clk) / 2;
> + clk_put(clk);
> +}
> --
> 1.9.1
>
>

Attachment: signature.asc
Description: Digital signature