Re: [PATCH v4 5/8] MIPS: ranchu: Add Ranchu as a new generic-based board

[Ralf Baechle <ralf@linux-mips.org>]

On Fri, Aug 18, 2017 at 03:08:57PM +0200, Aleksandar Markovic wrote:

(trimmed the cc list a bit by a number of people who probably couldn't
care less.)

> Provide amendments to the Mips generic platform framework so that
                            ^^^^
"MIPS" to keep the trademark lawyers happy.

> +CONFIG_STAGING=y

Sure you want to enable Greg's haunted house?

I haven't checked if it's actually needed but as a general rule leave it
disabled unless you have to.

> diff --git a/arch/mips/generic/Makefile b/arch/mips/generic/Makefile
> index 56b3ea5..14931f2 100644
> --- a/arch/mips/generic/Makefile
> +++ b/arch/mips/generic/Makefile
> @@ -14,4 +14,5 @@ obj-y += proc.o
>  
>  obj-$(CONFIG_YAMON_DT_SHIM)		+= yamon-dt.o
>  obj-$(CONFIG_LEGACY_BOARD_SEAD3)	+= board-sead3.o
> +obj-$(CONFIG_VIRT_BOARD_RANCHU)	+= board-ranchu.o
>  obj-$(CONFIG_KEXEC)			+= kexec.o
> diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
> new file mode 100644
> index 0000000..500874d
> --- /dev/null
> +++ b/arch/mips/generic/board-ranchu.c
> @@ -0,0 +1,78 @@
> +/*
> + * Copyright (C) 2017 Imagination Technologies Ltd.
> + * Author: Miodrag Dinic <miodrag.dinic@imgtec.com>
> + *
> + * 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 <linux/of_address.h>
> +
> +#include <asm/machine.h>
> +#include <asm/time.h>

You're using u64 / uint64_t below, so you need to #include <linux/types.h>
rather than playing the include lottery.

> +#define GOLDFISH_TIMER_LOW		0x00
> +#define GOLDFISH_TIMER_HIGH		0x04
> +
> +static __init uint64_t read_rtc_time(void __iomem *base)

The remainder of this file is using u64.  While the ISO C types are
acceptable these days, please try to be consistent in use of types.

> +{
> +	u64 time_low;
> +	u64 time_high;
> +
> +	time_low = readl(base + GOLDFISH_TIMER_LOW);
> +	time_high = readl(base + GOLDFISH_TIMER_HIGH);
> +
> +	return (time_high << 32) | time_low;
> +}

GCC used to generate pretty bad code from source like this.  I haven't
checked if it has improved but generally you want to avoid extending
data to 64 bit types for as long as possible, something like:

{
	u32 high, low;

	low = readl(base + GOLDFISH_TIMER_LOW);
	high = readl(base + GOLDFISH_TIMER_HIGH);

	return ((u64)high << 32) | low;
}

> +static __init unsigned int ranchu_measure_hpt_freq(void)
> +{
> +	u64 rtc_start, rtc_current, rtc_delta;
> +	unsigned int start, count;
> +	struct device_node *np;
> +	void __iomem *rtc_base;
> +
> +	np = of_find_compatible_node(NULL, NULL, "google,goldfish-rtc");
> +	if (!np)
> +		panic("%s(): Failed to find 'google,goldfish-rtc' dt node!",
> +		      __func__);
> +
> +	rtc_base = of_iomap(np, 0);
> +	if (!rtc_base)
> +		panic("%s(): Failed to ioremap Goldfish RTC base!", __func__);
> +
> +	/*
> +	 * poll the nanosecond resolution RTC for 1 second
> +	 * to calibrate the CPU frequency
> +	 */
> +	rtc_start = read_rtc_time(rtc_base);
> +	start = read_c0_count();
> +
> +	do {
> +		rtc_current = read_rtc_time(rtc_base);
> +		rtc_delta = rtc_current - rtc_start;
> +	} while (rtc_delta < NSEC_PER_SEC);
> +
> +	count = read_c0_count() - start;
> +
> +	count += 5000;	/* round */
> +	count -= count % 10000;
> +
> +	return count;
> +}
> +
> +static const struct of_device_id ranchu_of_match[];
> +
> +MIPS_MACHINE(ranchu) = {
> +	.matches = ranchu_of_match,
> +	.measure_hpt_freq = ranchu_measure_hpt_freq,
> +};
> +
> +static const struct of_device_id ranchu_of_match[] = {
> +	{
> +		.compatible = "mti,ranchu",
> +		.data = &__mips_mach_ranchu,
> +	},
> +};
> -- 
> 2.7.4
>