Re: [PATCH v3] MIPS: Make CP0 config registers readable via sysfs.

[David Daney <ddaney.cavm@gmail.com>]

On 12/13/2012 12:53 AM, Steven J. Hill wrote:
> From: "Steven J. Hill" <sjhill@mips.com>
>

Thanks Steven,

> Allow reading of CP0 config registers via sysfs for each core
> in the system. The registers will show up in sysfs at the path:
>
>     /sys/devices/system/cpu/cpuX/configX
>
> Only CP0 config registers 0 through 7 are currently supported.

That's not really a limitation, as those are the only ones that exist.


>
> Signed-off-by: Steven J. Hill <sjhill@mips.com>

One small change below, but otherwise,

Acked-by: David Daney <david.daney@cavium.com>

> ---
>   arch/mips/kernel/Makefile |    1 +
>   arch/mips/kernel/sysfs.c  |   68 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 69 insertions(+)
>   create mode 100644 arch/mips/kernel/sysfs.c
>
> diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
> index cc5eec6..0c3eb97 100644
> --- a/arch/mips/kernel/Makefile
> +++ b/arch/mips/kernel/Makefile
> @@ -97,6 +97,7 @@ obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o
>   obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event_mipsxx.o
>
>   obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
> +obj-y				+= sysfs.o
>
>   ifeq ($(CONFIG_CPU_MIPS32), y)
>   #
> diff --git a/arch/mips/kernel/sysfs.c b/arch/mips/kernel/sysfs.c
> new file mode 100644
> index 0000000..4f26349
> --- /dev/null
> +++ b/arch/mips/kernel/sysfs.c
> @@ -0,0 +1,68 @@
> +/*
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2012 MIPS Technologies, Inc.  All rights reserved.
> + */
> +#include <linux/init.h>
> +#include <linux/string.h>
> +#include <linux/cpu.h>
> +#include <linux/percpu.h>
> +
> +#include <asm/page.h>
> +
> +/* Convenience macro */
> +#define read_c0_config0()	read_c0_config()
> +
> +#define __BUILD_CP0_SYSFS(reg)					\
> +static ssize_t show_config##reg(struct device *dev,		\
> +		struct device_attribute *attr, char *buf)	\
> +{								\
> +	int n = snprintf(buf, PAGE_SIZE-2, "%x\n",		\
> +		read_c0_config##reg());				\
> +	return n;						\
> +}								\
> +static DEVICE_ATTR(config##reg, 0444, show_config##reg, NULL);

s/0444/S_IRUGO/


[...]

> +late_initcall(mips_sysfs_registers);
>

Why late_initcall?  I don't really have an objection, but unless there 
is a good reason, why not device_initcall?