Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [Patch] firmware: dmi_scan: split dmisubsystem from dmi-sysfs
From: Jean Delvare @ 2015-03-20  8:16 UTC (permalink / raw)
  To: Ivan.khoronzhuk
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	matt.fleming-ral2JQCrhuEAvxtiuMwx3w,
	ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, mikew-hpIqsD4AKlfQT0dZR+AlfA,
	dmidecode-devel-qX2TKyscuCcdnm+yROfE0A,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
	msalter-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <550B08E6.5050200-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>

Hi Ivan,

On Thu, 19 Mar 2015 19:35:34 +0200, Ivan.khoronzhuk wrote:
> On 19.03.15 17:30, Jean Delvare wrote:
> > Le Monday 16 March 2015 à 22:57 +0200, Ivan Khoronzhuk a écrit :
> >> Some utils, like dmidecode and smbios, need to access SMBIOS entry
> >> table area in order to get information like SMBIOS version, size, etc.
> >> Currently it's done via /dev/mem. But for situation when /dev/mem
> >> usage is disabled, the utils have to use dmi sysfs instead, which
> >> doesn't represent SMBIOS entry and adds code/delay redundancy when direct
> >> access for table is needed.
> >>
> >> So this patch creates dmi subsystem and adds SMBIOS entry point to allow
> >> utils in question to work correctly without /dev/mem. Also patch adds
> >> raw dmi table to simplify dmi table processing in user space, as were
> >> proposed by Jean Delvare.
> > "as was proposed" or even "as proposed" sounds more correct.
> >
> > BTW, which tree is your patch based on? I can't get it to apply cleanly
> > on top of any kernel version I tried. I adjusted the patch to my tree
> > but it means I'm not reviewing your code exactly. Please send patches
> > which can be applied on top of some recent vanilla tree (3.19 or 4.0-rc4
> > would be OK at the moment.)
> 
> Oh, sorry I forgot to mention, it's based on efi/next, but with consumption
> that Matt will remove old version:
>   85c83ea firmware: dmi-sysfs: Add SMBIOS entry point area attribute
> 
> As you remember, your propositions were slightly late,
> and patch had been applied on efi/next when you commented.

OK, I understand. Now I see the two patches I was missing, I'll pick
them so that I can apply your patch cleanly on top of my tree.

I would have appreciated to be Cc'd on these two patches, so I knew
they existed, and maybe I could even have commented on them.

> >> (...)
> >> @@ -118,6 +124,7 @@ static void dmi_table(u8 *buf,
> >>   }
> >>   
> >>   static phys_addr_t dmi_base;
> >> +static u8 *dmi_tb;
> > Where "tb" stands for... "table", but you couldn't use that because a
> > function by that name already exist, right? I can think of two less
> > cryptic ways to solve this: either you rename function dmi_table to,
> > say, dmi_decode_table (which would be a better name anyway IMHO), or you
> > name your variable dmi_table_p or dmi_raw_table. But "tb" is not
> > immediate enough to understand.
> 
> If others are OK, I'll better rename dmi_table function to 
> dmi_decode_table()
> (or maybe dmidecode_table :), joke)
> as it's more appropriate to what it's doing.
> And accordingly dmi_tb to dmi_table.

Yes, that's fine with me. The function rename should be a separate
patch for clarity.

> >> (...)
> >>   static int __init dmi_walk_early(void (*decode)(const struct dmi_header *,
> >>   		void *))
> >> @@ -476,6 +483,8 @@ static int __init dmi_present(const u8 *buf)
> >>   	if (memcmp(buf, "_SM_", 4) == 0 &&
> >>   	    buf[5] < 32 && dmi_checksum(buf, buf[5])) {
> >>   		smbios_ver = get_unaligned_be16(buf + 6);
> >> +		smbios_entry_point_size = buf[5];
> >> +		memcpy(smbios_entry_point, buf, smbios_entry_point_size);
> >>   
> >>   		/* Some BIOS report weird SMBIOS version, fix that up */
> >>   		switch (smbios_ver) {
> >> @@ -508,6 +517,8 @@ static int __init dmi_present(const u8 *buf)
> >>   					dmi_ver >> 8, dmi_ver & 0xFF,
> >>   					(dmi_ver < 0x0300) ? "" : ".x");
> >>   			} else {
> >> +				smbios_entry_point_size = 15;
> >> +				memcpy(smbios_entry_point, buf, 15);
> >>   				dmi_ver = (buf[14] & 0xF0) << 4 |
> >>   					   (buf[14] & 0x0F);
> >>   				pr_info("Legacy DMI %d.%d present.\n",
> >> @@ -535,6 +546,8 @@ static int __init dmi_smbios3_present(const u8 *buf)
> >>   		dmi_ver &= 0xFFFFFF;
> >>   		dmi_len = get_unaligned_le32(buf + 12);
> >>   		dmi_base = get_unaligned_le64(buf + 16);
> >> +		smbios_entry_point_size = buf[6];
> >> +		memcpy(smbios_entry_point, buf, smbios_entry_point_size);
> >>   
> >>   		/*
> >>   		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
> > This is inconsistent. You should either use smbios_entry_point_size as
> > the last argument to memcpy() in all 3 cases (my preference), or you
> > should use buf[5], 15 and buf[6] respectively, but not mix.
> 
> You probably meant "memcpy(smbios_entry_point, buf, 15)"

Yes.

> Just wanted to avoid redundant line break.

Line breaks are just fine, code consistency is more important.

> >> (...)
> >> +	if (!smbios_entry_point_size || !dmi_available) {
> > I already mentioned in a previous review that I don't think you need to
> > check for !dmi_available, and you said you agreed.
> 
> No.
> Previously only smbios entry point was exported.
> Now DMI table was added.
> smbios_entry_point_size doesn't mean DMI table is present.

Thanks for the explanation, I understand the reasoning now. I can see
how smbios_entry_point_size could be set while dmi_available would
not. But I can't see how dmi_available could be set and
smbios_entry_point_size would not. So I think checking for
dmi_available is enough?

> >> (...)
> >> +		goto err;
> >> +	}
> >> +
> >> +	/* Set up dmi directory at /sys/firmware/dmi */
> >> +	dmi_kobj = kobject_create_and_add("dmi", firmware_kobj);
> >> +	if (!dmi_kobj)
> >> +		goto err;
> >> +
> >> +	bin_attr_smbios_entry_point.size = smbios_entry_point_size;
> >> +	ret = sysfs_create_bin_file(dmi_kobj, &bin_attr_smbios_entry_point);
> >> +	if (ret)
> >> +		goto err;
> >> +
> >> +	if (!dmi_tb) {
> > I don't like this. You should know which of this function and dmi_walk()
> > is called first. If you don't, then how can you guarantee that a race
> > condition can't happen?
> 
> Shit.
> Maybe better to leave dmi_walk as it was
> and map it only here.

For the time being, yes, that might be better. We can always optimize
it later in a separate patch.

> > This makes me wonder if that code wouldn't rather go in
> > dmi_scan_machine() rather than a separate function.

> >> (...)
> >> +		dmi_tb = dmi_remap(dmi_base, dmi_len);
> >> +		if (!dmi_tb)
> >> +			goto err;
> >> +	}
> >> +
> >> +	bin_attr_dmi_table.size = dmi_len;
> >> +	ret = sysfs_create_bin_file(dmi_kobj, &bin_attr_dmi_table);
> >> +	if (ret)
> >> +		goto err;
> >> +
> >> +	return 0;
> >> +err:
> >> +	pr_err("dmi: Firmware registration failed.\n");
> >
> > I said in a previous review that files that have been created should be
> > explicitly deleted, and I still think so.
> 
> I dislike it, but if you insist I'll do this.

I'm only repeating something I was told myself when submitting that
kind of code long ago. Almost all other drivers seem to be cleaning up
such files explicitly, just look at the firmware drivers, efivars for
example.

If you don't want to do it, I don't really mind, that's an error path
that most probably nobody will ever walk unless explicitly testing it.
But then if something breaks, you'll be asked to fix it.

> >> (...)
> >> +	kobject_del(dmi_kobj);
> >> +	kobject_put(dmi_kobj);
> > I think you also need to explicitly set dmi_kobj to NULL here, otherwise
> > dmi-sysfs will try to use an object which no longer exists.
> 
> Yes.
> 
> > An alternative approach would be to leave dmi_kobj available even if the
> > binary files could not be created. As this case is very unlikely to
> > happen, I don't care which way you choose.
> 
> I also thought about such approach. But imaged a situation when DMI 
> catalog is
> empty and no one uses dmi-sysfs (which probably is more frequently), decided
> to delete it. So dmi_kobj = 0.

Fine with me (but = NULL, not = 0.)

Thanks,
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply

* Re: [PATCH v7 tip 2/8] tracing: attach BPF programs to kprobes
From: Masami Hiramatsu @ 2015-03-20  8:35 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Ingo Molnar, Steven Rostedt, Namhyung Kim,
	Arnaldo Carvalho de Melo, Jiri Olsa, David S. Miller,
	Daniel Borkmann, Peter Zijlstra, linux-api-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1426542584-9406-3-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

(2015/03/17 6:49), Alexei Starovoitov wrote:
> User interface:
> struct perf_event_attr attr = {.type = PERF_TYPE_TRACEPOINT, .config = event_id, ...};
> event_fd = perf_event_open(&attr,...);
> ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> 
> prog_fd is a file descriptor associated with BPF program previously loaded.
> event_id is an ID of created kprobe
> 
> close(event_fd) - automatically detaches BPF program from it
> 
> BPF programs can call in-kernel helper functions to:
> - lookup/update/delete elements in maps
> - probe_read - wraper of probe_kernel_read() used to access any kernel
>   data structures
> 
> BPF programs receive 'struct pt_regs *' as an input
> ('struct pt_regs' is architecture dependent)
> 
> Note, kprobes are _not_ a stable kernel ABI, so bpf programs attached to
> kprobes must be recompiled for every kernel version and user must supply correct
> LINUX_VERSION_CODE in attr.kern_version during bpf_prog_load() call.

For the part of kprobe event code, it seems OK to me. :)

Thanks,

> 
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> ---
>  include/linux/ftrace_event.h    |   14 +++++
>  include/uapi/linux/bpf.h        |    3 +
>  include/uapi/linux/perf_event.h |    1 +
>  kernel/bpf/syscall.c            |    7 ++-
>  kernel/events/core.c            |   59 +++++++++++++++++++
>  kernel/trace/Makefile           |    1 +
>  kernel/trace/bpf_trace.c        |  119 +++++++++++++++++++++++++++++++++++++++
>  kernel/trace/trace_kprobe.c     |   10 +++-
>  8 files changed, 212 insertions(+), 2 deletions(-)
>  create mode 100644 kernel/trace/bpf_trace.c
> 
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index c674ee8f7fca..0aa535bc9f05 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -13,6 +13,7 @@ struct trace_array;
>  struct trace_buffer;
>  struct tracer;
>  struct dentry;
> +struct bpf_prog;
>  
>  struct trace_print_flags {
>  	unsigned long		mask;
> @@ -252,6 +253,7 @@ enum {
>  	TRACE_EVENT_FL_WAS_ENABLED_BIT,
>  	TRACE_EVENT_FL_USE_CALL_FILTER_BIT,
>  	TRACE_EVENT_FL_TRACEPOINT_BIT,
> +	TRACE_EVENT_FL_KPROBE_BIT,
>  };
>  
>  /*
> @@ -265,6 +267,7 @@ enum {
>   *                     it is best to clear the buffers that used it).
>   *  USE_CALL_FILTER - For ftrace internal events, don't use file filter
>   *  TRACEPOINT    - Event is a tracepoint
> + *  KPROBE        - Event is a kprobe
>   */
>  enum {
>  	TRACE_EVENT_FL_FILTERED		= (1 << TRACE_EVENT_FL_FILTERED_BIT),
> @@ -274,6 +277,7 @@ enum {
>  	TRACE_EVENT_FL_WAS_ENABLED	= (1 << TRACE_EVENT_FL_WAS_ENABLED_BIT),
>  	TRACE_EVENT_FL_USE_CALL_FILTER	= (1 << TRACE_EVENT_FL_USE_CALL_FILTER_BIT),
>  	TRACE_EVENT_FL_TRACEPOINT	= (1 << TRACE_EVENT_FL_TRACEPOINT_BIT),
> +	TRACE_EVENT_FL_KPROBE		= (1 << TRACE_EVENT_FL_KPROBE_BIT),
>  };
>  
>  struct ftrace_event_call {
> @@ -303,6 +307,7 @@ struct ftrace_event_call {
>  #ifdef CONFIG_PERF_EVENTS
>  	int				perf_refcount;
>  	struct hlist_head __percpu	*perf_events;
> +	struct bpf_prog			*prog;
>  
>  	int	(*perf_perm)(struct ftrace_event_call *,
>  			     struct perf_event *);
> @@ -548,6 +553,15 @@ event_trigger_unlock_commit_regs(struct ftrace_event_file *file,
>  		event_triggers_post_call(file, tt);
>  }
>  
> +#ifdef CONFIG_BPF_SYSCALL
> +unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx);
> +#else
> +static inline unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
> +{
> +	return 1;
> +}
> +#endif
> +
>  enum {
>  	FILTER_OTHER = 0,
>  	FILTER_STATIC_STRING,
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 45da7ec7d274..4486d36d2e9e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -118,6 +118,7 @@ enum bpf_map_type {
>  enum bpf_prog_type {
>  	BPF_PROG_TYPE_UNSPEC,
>  	BPF_PROG_TYPE_SOCKET_FILTER,
> +	BPF_PROG_TYPE_KPROBE,
>  };
>  
>  /* flags for BPF_MAP_UPDATE_ELEM command */
> @@ -151,6 +152,7 @@ union bpf_attr {
>  		__u32		log_level;	/* verbosity level of verifier */
>  		__u32		log_size;	/* size of user buffer */
>  		__aligned_u64	log_buf;	/* user supplied buffer */
> +		__u32		kern_version;	/* checked when type=kprobe */
>  	};
>  } __attribute__((aligned(8)));
>  
> @@ -162,6 +164,7 @@ enum bpf_func_id {
>  	BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(&map, &key) */
>  	BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
>  	BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
> +	BPF_FUNC_probe_read,      /* int bpf_probe_read(void *dst, int size, void *src) */
>  	__BPF_FUNC_MAX_ID,
>  };
>  
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 3c8b45de57ec..ad4dade2a502 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -382,6 +382,7 @@ struct perf_event_attr {
>  #define PERF_EVENT_IOC_SET_OUTPUT	_IO ('$', 5)
>  #define PERF_EVENT_IOC_SET_FILTER	_IOW('$', 6, char *)
>  #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
> +#define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
>  
>  enum perf_event_ioc_flags {
>  	PERF_IOC_FLAG_GROUP		= 1U << 0,
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 536edc2be307..504c10b990ef 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -16,6 +16,7 @@
>  #include <linux/file.h>
>  #include <linux/license.h>
>  #include <linux/filter.h>
> +#include <linux/version.h>
>  
>  static LIST_HEAD(bpf_map_types);
>  
> @@ -467,7 +468,7 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
>  }
>  
>  /* last field in 'union bpf_attr' used by this command */
> -#define	BPF_PROG_LOAD_LAST_FIELD log_buf
> +#define	BPF_PROG_LOAD_LAST_FIELD kern_version
>  
>  static int bpf_prog_load(union bpf_attr *attr)
>  {
> @@ -492,6 +493,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>  	if (attr->insn_cnt >= BPF_MAXINSNS)
>  		return -EINVAL;
>  
> +	if (type == BPF_PROG_TYPE_KPROBE &&
> +	    attr->kern_version != LINUX_VERSION_CODE)
> +		return -EINVAL;
> +
>  	/* plain bpf_prog allocation */
>  	prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
>  	if (!prog)
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 2709063eb26b..3a45e7f6b2df 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -42,6 +42,8 @@
>  #include <linux/module.h>
>  #include <linux/mman.h>
>  #include <linux/compat.h>
> +#include <linux/bpf.h>
> +#include <linux/filter.h>
>  
>  #include "internal.h"
>  
> @@ -3402,6 +3404,7 @@ errout:
>  }
>  
>  static void perf_event_free_filter(struct perf_event *event);
> +static void perf_event_free_bpf_prog(struct perf_event *event);
>  
>  static void free_event_rcu(struct rcu_head *head)
>  {
> @@ -3411,6 +3414,7 @@ static void free_event_rcu(struct rcu_head *head)
>  	if (event->ns)
>  		put_pid_ns(event->ns);
>  	perf_event_free_filter(event);
> +	perf_event_free_bpf_prog(event);
>  	kfree(event);
>  }
>  
> @@ -3923,6 +3927,7 @@ static inline int perf_fget_light(int fd, struct fd *p)
>  static int perf_event_set_output(struct perf_event *event,
>  				 struct perf_event *output_event);
>  static int perf_event_set_filter(struct perf_event *event, void __user *arg);
> +static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
>  
>  static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
>  {
> @@ -3976,6 +3981,9 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
>  	case PERF_EVENT_IOC_SET_FILTER:
>  		return perf_event_set_filter(event, (void __user *)arg);
>  
> +	case PERF_EVENT_IOC_SET_BPF:
> +		return perf_event_set_bpf_prog(event, arg);
> +
>  	default:
>  		return -ENOTTY;
>  	}
> @@ -6436,6 +6444,49 @@ static void perf_event_free_filter(struct perf_event *event)
>  	ftrace_profile_free_filter(event);
>  }
>  
> +static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
> +{
> +	struct bpf_prog *prog;
> +
> +	if (event->attr.type != PERF_TYPE_TRACEPOINT)
> +		return -EINVAL;
> +
> +	if (event->tp_event->prog)
> +		return -EEXIST;
> +
> +	if (!(event->tp_event->flags & TRACE_EVENT_FL_KPROBE))
> +		/* bpf programs can only be attached to kprobes */
> +		return -EINVAL;
> +
> +	prog = bpf_prog_get(prog_fd);
> +	if (IS_ERR(prog))
> +		return PTR_ERR(prog);
> +
> +	if (prog->aux->prog_type != BPF_PROG_TYPE_KPROBE) {
> +		/* valid fd, but invalid bpf program type */
> +		bpf_prog_put(prog);
> +		return -EINVAL;
> +	}
> +
> +	event->tp_event->prog = prog;
> +
> +	return 0;
> +}
> +
> +static void perf_event_free_bpf_prog(struct perf_event *event)
> +{
> +	struct bpf_prog *prog;
> +
> +	if (!event->tp_event)
> +		return;
> +
> +	prog = event->tp_event->prog;
> +	if (prog) {
> +		event->tp_event->prog = NULL;
> +		bpf_prog_put(prog);
> +	}
> +}
> +
>  #else
>  
>  static inline void perf_tp_register(void)
> @@ -6451,6 +6502,14 @@ static void perf_event_free_filter(struct perf_event *event)
>  {
>  }
>  
> +static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
> +{
> +	return -ENOENT;
> +}
> +
> +static void perf_event_free_bpf_prog(struct perf_event *event)
> +{
> +}
>  #endif /* CONFIG_EVENT_TRACING */
>  
>  #ifdef CONFIG_HAVE_HW_BREAKPOINT
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index 98f26588255e..c575a300103b 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_EVENT_TRACING) += trace_event_perf.o
>  endif
>  obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o
>  obj-$(CONFIG_EVENT_TRACING) += trace_events_trigger.o
> +obj-$(CONFIG_BPF_SYSCALL) += bpf_trace.o
>  obj-$(CONFIG_KPROBE_EVENT) += trace_kprobe.o
>  obj-$(CONFIG_TRACEPOINTS) += power-traces.o
>  ifeq ($(CONFIG_PM),y)
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> new file mode 100644
> index 000000000000..ba95b131082c
> --- /dev/null
> +++ b/kernel/trace/bpf_trace.c
> @@ -0,0 +1,119 @@
> +/* Copyright (c) 2011-2015 PLUMgrid, http://plumgrid.com
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/bpf.h>
> +#include <linux/filter.h>
> +#include <linux/uaccess.h>
> +#include "trace.h"
> +
> +static DEFINE_PER_CPU(int, bpf_prog_active);
> +
> +unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
> +{
> +	unsigned int ret;
> +	int cpu;
> +
> +	if (in_nmi()) /* not supported yet */
> +		return 1;
> +
> +	preempt_disable();
> +
> +	cpu = raw_smp_processor_id();
> +	if (unlikely(per_cpu(bpf_prog_active, cpu)++ != 0)) {
> +		/* since some bpf program is already running on this cpu,
> +		 * don't call into another bpf program (same or different)
> +		 * and don't send kprobe event into ring-buffer,
> +		 * so return zero here
> +		 */
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	rcu_read_lock();
> +	ret = BPF_PROG_RUN(prog, ctx);
> +	rcu_read_unlock();
> +
> + out:
> +	per_cpu(bpf_prog_active, cpu)--;
> +	preempt_enable();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(trace_call_bpf);
> +
> +static u64 bpf_probe_read(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
> +{
> +	void *dst = (void *) (long) r1;
> +	int size = (int) r2;
> +	void *unsafe_ptr = (void *) (long) r3;
> +
> +	return probe_kernel_read(dst, unsafe_ptr, size);
> +}
> +
> +static struct bpf_func_proto kprobe_prog_funcs[] = {
> +	[BPF_FUNC_probe_read] = {
> +		.func = bpf_probe_read,
> +		.gpl_only = true,
> +		.ret_type = RET_INTEGER,
> +		.arg1_type = ARG_PTR_TO_STACK,
> +		.arg2_type = ARG_CONST_STACK_SIZE,
> +		.arg3_type = ARG_ANYTHING,
> +	},
> +};
> +
> +static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func_id)
> +{
> +	switch (func_id) {
> +	case BPF_FUNC_map_lookup_elem:
> +		return &bpf_map_lookup_elem_proto;
> +	case BPF_FUNC_map_update_elem:
> +		return &bpf_map_update_elem_proto;
> +	case BPF_FUNC_map_delete_elem:
> +		return &bpf_map_delete_elem_proto;
> +	default:
> +		if (func_id < 0 || func_id >= ARRAY_SIZE(kprobe_prog_funcs))
> +			return NULL;
> +		return &kprobe_prog_funcs[func_id];
> +	}
> +}
> +
> +/* bpf+kprobe programs can access fields of 'struct pt_regs' */
> +static bool kprobe_prog_is_valid_access(int off, int size, enum bpf_access_type type)
> +{
> +	/* check bounds */
> +	if (off < 0 || off >= sizeof(struct pt_regs))
> +		return false;
> +
> +	/* only read is allowed */
> +	if (type != BPF_READ)
> +		return false;
> +
> +	/* disallow misaligned access */
> +	if (off % size != 0)
> +		return false;
> +
> +	return true;
> +}
> +
> +static struct bpf_verifier_ops kprobe_prog_ops = {
> +	.get_func_proto = kprobe_prog_func_proto,
> +	.is_valid_access = kprobe_prog_is_valid_access,
> +};
> +
> +static struct bpf_prog_type_list kprobe_tl = {
> +	.ops = &kprobe_prog_ops,
> +	.type = BPF_PROG_TYPE_KPROBE,
> +};
> +
> +static int __init register_kprobe_prog_ops(void)
> +{
> +	bpf_register_prog_type(&kprobe_tl);
> +	return 0;
> +}
> +late_initcall(register_kprobe_prog_ops);
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index d73f565b4e06..dc3462507d7c 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1134,11 +1134,15 @@ static void
>  kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
>  {
>  	struct ftrace_event_call *call = &tk->tp.call;
> +	struct bpf_prog *prog = call->prog;
>  	struct kprobe_trace_entry_head *entry;
>  	struct hlist_head *head;
>  	int size, __size, dsize;
>  	int rctx;
>  
> +	if (prog && !trace_call_bpf(prog, regs))
> +		return;
> +
>  	head = this_cpu_ptr(call->perf_events);
>  	if (hlist_empty(head))
>  		return;
> @@ -1165,11 +1169,15 @@ kretprobe_perf_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
>  		    struct pt_regs *regs)
>  {
>  	struct ftrace_event_call *call = &tk->tp.call;
> +	struct bpf_prog *prog = call->prog;
>  	struct kretprobe_trace_entry_head *entry;
>  	struct hlist_head *head;
>  	int size, __size, dsize;
>  	int rctx;
>  
> +	if (prog && !trace_call_bpf(prog, regs))
> +		return;
> +
>  	head = this_cpu_ptr(call->perf_events);
>  	if (hlist_empty(head))
>  		return;
> @@ -1286,7 +1294,7 @@ static int register_kprobe_event(struct trace_kprobe *tk)
>  		kfree(call->print_fmt);
>  		return -ENODEV;
>  	}
> -	call->flags = 0;
> +	call->flags = TRACE_EVENT_FL_KPROBE;
>  	call->class->reg = kprobe_register;
>  	call->data = tk;
>  	ret = trace_add_event_call(call);
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org

^ permalink raw reply

* Re: [PATCH 1/1] Add virtio-input driver.
From: Gerd Hoffmann @ 2015-03-20  9:48 UTC (permalink / raw)
  To: David Herrmann
  Cc: virtio-dev-sDuHXQ4OtrM4h7I2RyI4rWD2FQJk+8+b,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	mst-H+wXaHxf7aLQT0dZR+AlfA, Rusty Russell, open list,
	open list:ABI/API, Dmitry Torokhov
In-Reply-To: <CANq1E4TDj4pq3J_BVc=Yuzo5dVR=QcNexVUqaqwjg7Qi5_xX4Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


  Hi,

> > +static int virtinput_send_status(struct virtio_input *vi,
> > +                                u16 type, u16 code, s32 value)
> > +{
> > +       struct virtio_input_event *stsbuf;
> > +       struct scatterlist sg[1];
> > +
> > +       stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
> > +       if (!stsbuf)
> > +               return -ENOMEM;
> > +
> > +       stsbuf->type  = cpu_to_le16(type);
> > +       stsbuf->code  = cpu_to_le16(code);
> > +       stsbuf->value = cpu_to_le32(value);
> > +       sg_init_one(sg, stsbuf, sizeof(*stsbuf));
> > +       virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
> > +       virtqueue_kick(vi->sts);
> 
> GFP_ATOMIC, eww. But everyone does that for input_event() callbacks..

Yea, did it this way because I saw it elsewhere.

> we should fix that for user-space input one day.

Sounds like I have to use GFP_ATOMIC and can't switch to GFP_KERNEL,
correct?

> > +       size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
> 
> abs.resolution is missing. Please add it, we really also need to add
> it to uinput one day.

Ok.  How should I handle cases where the resolution is either not known
or not fixed?  Just leave it zero?

> > +       vi->idev->name = vi->name;
> > +       vi->idev->phys = vi->phys;
> 
> Can you set vi->idev->uniq to the virtio-bus path?
> 
> > +       vi->idev->id.bustype = BUS_VIRTUAL;
> > +       vi->idev->id.vendor  = 0x0001;
> > +       vi->idev->id.product = 0x0001;
> > +       vi->idev->id.version = 0x0100;
> 
> Please don't hardcode those. All user-space based interaction with
> input-devices relies on those IDs. Can we retrieve it from the host
> just like the name?

Yes, we can.

There will be emulated devices, i.e. the input coming from
vnc/gtk/whatever will be sent to the virtio devices (instead of ps/2 or
usb).  For these we should probably have fixed IDs per device.  There
are keyboard/mouse/tablet at the moment.  Suggestions how to pick IDs?

There will also be pass-through support, i.e. qemu
opening /dev/input/event<nr> and forwarding everything to the guest.
How should that be handled best?  Copy all four from the host?  Even
though the bustype is BUS_USB?  Not sure this actually improves things
because the guest can match the device, or whenever this confuses apps
due to BUS_USB being applied to virtio devices ...

cheers,
  Gerd

^ permalink raw reply

* Re: [PATCH 1/1] Add virtio-input driver.
From: Gerd Hoffmann @ 2015-03-20  9:54 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, virtio-dev, virtualization, mst, Rusty Russell,
	open list, open list:ABI/API
In-Reply-To: <20150319162704.GE30732@dtor-ws>

  Hi,

> > > +static ssize_t serial_show(struct device *dev,
> > > +                          struct device_attribute *attr, char *buf)
> > > +{
> > > +       struct input_dev *idev = to_input_dev(dev);
> > > +       struct virtio_input *vi = input_get_drvdata(idev);
> > > +       return sprintf(buf, "%s\n", vi->serial);
> > > +}
> > > +static DEVICE_ATTR_RO(serial);
> 
> What is serial? Serial number?

Yes.  You can (optionally) configure a serial number on the host side,
and if that is the case it'll show up here.

> > Can you set vi->idev->uniq to the virtio-bus path?
> 
> No, uniq can't be phys as phys is unique within the system while uniq is
> like serial number or UUID and should never repeat.

Ok, so I guess I should just fill uniq with serial (if present).

cheers,
  Gerd

^ permalink raw reply

* Re: [PATCH 1/1] Add virtio-input driver.
From: David Herrmann @ 2015-03-20  9:55 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: virtio-dev-sDuHXQ4OtrM4h7I2RyI4rWD2FQJk+8+b,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	mst-H+wXaHxf7aLQT0dZR+AlfA, Rusty Russell, open list,
	open list:ABI/API, Dmitry Torokhov
In-Reply-To: <1426844885.32097.36.camel-3OfP5uLMi4C46o+2HkPkLj4oCIwMql/M@public.gmane.org>

Hi

On Fri, Mar 20, 2015 at 10:48 AM, Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
>   Hi,
>
>> > +static int virtinput_send_status(struct virtio_input *vi,
>> > +                                u16 type, u16 code, s32 value)
>> > +{
>> > +       struct virtio_input_event *stsbuf;
>> > +       struct scatterlist sg[1];
>> > +
>> > +       stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
>> > +       if (!stsbuf)
>> > +               return -ENOMEM;
>> > +
>> > +       stsbuf->type  = cpu_to_le16(type);
>> > +       stsbuf->code  = cpu_to_le16(code);
>> > +       stsbuf->value = cpu_to_le32(value);
>> > +       sg_init_one(sg, stsbuf, sizeof(*stsbuf));
>> > +       virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
>> > +       virtqueue_kick(vi->sts);
>>
>> GFP_ATOMIC, eww. But everyone does that for input_event() callbacks..
>
> Yea, did it this way because I saw it elsewhere.
>
>> we should fix that for user-space input one day.
>
> Sounds like I have to use GFP_ATOMIC and can't switch to GFP_KERNEL,
> correct?

You cannot use GFP_KERNEL in this context, correct. GFP_ATOMIC is also
what all HID backends do, so it's fine here.

>> > +       size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
>> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
>> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
>> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
>> > +       virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
>>
>> abs.resolution is missing. Please add it, we really also need to add
>> it to uinput one day.
>
> Ok.  How should I handle cases where the resolution is either not known
> or not fixed?  Just leave it zero?

Leave it 0, which is what you already do by not assigning it.

>> > +       vi->idev->name = vi->name;
>> > +       vi->idev->phys = vi->phys;
>>
>> Can you set vi->idev->uniq to the virtio-bus path?
>>
>> > +       vi->idev->id.bustype = BUS_VIRTUAL;
>> > +       vi->idev->id.vendor  = 0x0001;
>> > +       vi->idev->id.product = 0x0001;
>> > +       vi->idev->id.version = 0x0100;
>>
>> Please don't hardcode those. All user-space based interaction with
>> input-devices relies on those IDs. Can we retrieve it from the host
>> just like the name?
>
> Yes, we can.
>
> There will be emulated devices, i.e. the input coming from
> vnc/gtk/whatever will be sent to the virtio devices (instead of ps/2 or
> usb).  For these we should probably have fixed IDs per device.  There
> are keyboard/mouse/tablet at the moment.  Suggestions how to pick IDs?
>
> There will also be pass-through support, i.e. qemu
> opening /dev/input/event<nr> and forwarding everything to the guest.
> How should that be handled best?  Copy all four from the host?  Even
> though the bustype is BUS_USB?  Not sure this actually improves things
> because the guest can match the device, or whenever this confuses apps
> due to BUS_USB being applied to virtio devices ...

Lemme give an example: We have databases in user-space, that allow
applications to figure out the mouse DPI values of a device. Those
databases match on all four, bus+vid+pid+ver (sometimes even more,
like name and dmi). If one of those is not forwarded, it will not be
detected.

I'd like to see all four forwarded from the host. I'd be fine with
"bus" being set to VIRTUAL, but I'm not sure why that would be a good
thing to do?

Thanks
David

^ permalink raw reply

* Re: [PATCH 1/1] Add virtio-input driver.
From: Gerd Hoffmann @ 2015-03-20 10:28 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtio-dev, open list, open list:ABI/API, virtualization
In-Reply-To: <20150319123940-mutt-send-email-mst@redhat.com>

  Hi,

> > +static int virtinput_send_status(struct virtio_input *vi,
> > +				 u16 type, u16 code, s32 value)
> > +{
> > +	struct virtio_input_event *stsbuf;
> > +	struct scatterlist sg[1];
> > +
> > +	stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
> > +	if (!stsbuf)
> > +		return -ENOMEM;
> 
> Does this return an error to userspace?
> If so it's not a good idea I think, GFP_ATOMIC failures are
> transient conditions and should not be reported
> to userspace.
> Can use GFP_KERNEL here?

Waiting for an answer from the ioput guys here.

> > +
> > +	stsbuf->type  = cpu_to_le16(type);
> > +	stsbuf->code  = cpu_to_le16(code);
> > +	stsbuf->value = cpu_to_le32(value);
> > +	sg_init_one(sg, stsbuf, sizeof(*stsbuf));
> > +	virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
> 
> This can fail if queue is full. What prevents this from happening?

Nothing.  It's highly unlikely though given the throughput we have for
input devices, not sure it is that useful to put too much effort into
this.  Except for freeing stsbuf in the error case.

> > +	virtqueue_kick(vi->sts);
> 
> Also what prevents multiple virtinput_send_status calls
> from racing with each other? Is there locking at a higher level?

I think input_dev->event_lock does this.

> > +static void virtinput_recv_status(struct virtqueue *vq)
> > +{
> > +	struct virtio_input *vi = vq->vdev->priv;
> > +	struct virtio_input_event *stsbuf;
> > +	unsigned int len;
> > +
> > +	while ((stsbuf = virtqueue_get_buf(vi->sts, &len)) != NULL)
> 
> looks like this get_buf can race with add above.

Yes, it can.

> Need some locking.

I'll add it.

> Also pls avoid != NULL, removing it makes code less verbose.
> 
> > +		kfree(stsbuf);
> > +	virtqueue_kick(vq);
> 
> Why are you kicking here?

Hmm, it is pointless indeed.

> > +	if (select == VIRTIO_INPUT_CFG_EV_BITS)
> > +		set_bit(subsel, vi->idev->evbit);
> > +	for (bit = 0; bit < bitcount; bit++) {
> > +		if ((bit % 8) == 0)
> > +			virtio_cread(vi->vdev, struct virtio_input_config,
> > +				     u.bitmap[bit/8], &cfg);
> 
> coding style violations above. you need spaces around ops like / and *.
> Please run checkpatch.pl
> 
> > +		if (cfg & (1 << (bit % 8)))
> > +			set_bit(bit, bits);
> 
> what if not set? does something clear the mask?

kzalloc?

> > +	}
> 
> doesn't above just implement bitmap_copy or bitmap_or?

Not fully sure how bitmaps are defined.  virtio has a stream of bytes,
first byte carries bits 0-7, second 8-15 etc.  linux kernel bitmaps ops
are operating on longs, and native byteorder longs would be something
else ...

> > +	size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
> > +	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
> > +	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
> > +	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
> > +	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
> 
> you read le field into u32 value.
> Please run sparse on this code. you will get a ton
> of warnings. Same error appears elsewhere.

Indeed.  IIRC that wasn't the case a while back.  Guess those bitwise
annotations have been added with the virtio 1.0 patches?

In any case I'll fix it up.

> > +static int virtinput_probe(struct virtio_device *vdev)
> > +{
> > +	struct virtio_input *vi;
> > +	size_t size;
> > +	int abs, err;
> 
> How about checking VERSION_1 and bailing out of not there?

I don't think this is needed.  There isn't a hard dependency on virtio
1.0.  It's just that config space is relatively large and because of
that I want it be 1.0 on the host (qemu) side to not allocate large
portions of I/O address space for the legacy virtio pci bar.

> > +	vi->idev->name = vi->name;
> > +	vi->idev->phys = vi->phys;
> > +	vi->idev->id.bustype = BUS_VIRTUAL;
> > +	vi->idev->id.vendor  = 0x0001;
> > +	vi->idev->id.product = 0x0001;
> > +	vi->idev->id.version = 0x0100;
> 
> Add comments explaining why these #s make sense?

See other subthread, will be changed to be host-provided (like name).

> > +	err = input_register_device(vi->idev);
> 
> Once you do this, virtinput_status can get called,
> and that will kick, correct?

Correct.

> If so, you must call device_ready before this.

Ok.

> > +	if (err)
> > +		goto out4;
> > +
> > +	return 0;
> > +
> > +out4:
> > +	input_free_device(vi->idev);
> > +out3:
> > +	vdev->config->del_vqs(vdev);
> > +out2:
> > +	kfree(vi);
> > +out1:
> > +	return err;
> 
> free on error is out of order with initialization.
> Might lead to leaks or other bugs.
> Also - can you name labels something sensible pls?
> out is usually for exiting on success too...
> E.g. out4 -> err_register etc.

Will fix.

> > +static void virtinput_remove(struct virtio_device *vdev)
> > +{
> > +	struct virtio_input *vi = vdev->priv;
> > +
> > +	input_unregister_device(vi->idev);
> > +	vdev->config->reset(vdev);
> 
> You don't really need a reset if you just to del_vqs.
> People do this if they want to prevent interrupts
> without deleting vqs.

Ok.

> > +	vdev->config->del_vqs(vdev);
> > +	kfree(vi);
> 
> free_device seems to be missing?

Indeed, good catch.

thanks,
  Gerd

^ permalink raw reply

* Re: [PATCH 1/1] Add virtio-input driver.
From: Gerd Hoffmann @ 2015-03-20 10:36 UTC (permalink / raw)
  To: David Herrmann
  Cc: virtio-dev-sDuHXQ4OtrM4h7I2RyI4rWD2FQJk+8+b,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	mst-H+wXaHxf7aLQT0dZR+AlfA, Rusty Russell, open list,
	open list:ABI/API, Dmitry Torokhov
In-Reply-To: <CANq1E4QLPSK6NVeEx6yihYPdF-XPpXx4rKv0deHwX+s2RzFHCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

  Hi,

> > There will also be pass-through support, i.e. qemu
> > opening /dev/input/event<nr> and forwarding everything to the guest.
> > How should that be handled best?  Copy all four from the host?  Even
> > though the bustype is BUS_USB?  Not sure this actually improves things
> > because the guest can match the device, or whenever this confuses apps
> > due to BUS_USB being applied to virtio devices ...
> 
> Lemme give an example: We have databases in user-space, that allow
> applications to figure out the mouse DPI values of a device. Those
> databases match on all four, bus+vid+pid+ver (sometimes even more,
> like name and dmi). If one of those is not forwarded, it will not be
> detected.

Ok, so forward as much as possible.

> I'd like to see all four forwarded from the host. I'd be fine with
> "bus" being set to VIRTUAL, but I'm not sure why that would be a good
> thing to do?

I think for the emulated devices it's fine to use VIRTUAL.

For the passthrough case suspected we could confuse apps because ->phys
points to a virtio device whereas ->type says "I'm usb".

But at least the device database probably doesn't care much about the
physical path I guess, because the mouse is the same no matter which usb
port I plug it in, correct?

cheers,
  Gerd

^ permalink raw reply

* Re: [PATCH 1/1] Add virtio-input driver.
From: David Herrmann @ 2015-03-20 10:43 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: virtio-dev-sDuHXQ4OtrM4h7I2RyI4rWD2FQJk+8+b,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	mst-H+wXaHxf7aLQT0dZR+AlfA, Rusty Russell, open list,
	open list:ABI/API, Dmitry Torokhov
In-Reply-To: <1426847799.32097.66.camel-3OfP5uLMi4C46o+2HkPkLj4oCIwMql/M@public.gmane.org>

Hi

On Fri, Mar 20, 2015 at 11:36 AM, Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> I'd like to see all four forwarded from the host. I'd be fine with
>> "bus" being set to VIRTUAL, but I'm not sure why that would be a good
>> thing to do?
>
> I think for the emulated devices it's fine to use VIRTUAL.

Yes, on the host side just use BUS_VIRTUAL if you don't have a real bus to set.

> For the passthrough case suspected we could confuse apps because ->phys
> points to a virtio device whereas ->type says "I'm usb".

That's not an issue. The "phys" field hasn't been standardized in any
way that I'm aware of (except on a per-driver basis, maybe).

> But at least the device database probably doesn't care much about the
> physical path I guess, because the mouse is the same no matter which usb
> port I plug it in, correct?

Correct!

Thanks
David

^ permalink raw reply

* [PATCH v4] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Martin Kepplinger @ 2015-03-20 11:20 UTC (permalink / raw)
  To: mark.rutland, robh+dt, Pawel.Moll, ijc+devicetree, galak,
	dmitry.torokhov, alexander.stein
  Cc: hadess, akpm, gregkh, linux-api, devicetree, linux-input,
	linux-kernel, Martin Kepplinger, Christoph Muellner

From: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>

The MMA8653FC is a low-power, three-axis, capacitive micromachined
accelerometer with 10 bits of resolution with flexible user-programmable
options.

Embedded interrupt functions enable overall power savings, by relieving the
host processor from continuously polling data, for example using the poll()
system call.

The device can be configured to generate wake-up interrupt signals from any
combination of the configurable embedded functions, enabling the MMA8653FC
to monitor events while remaining in a low-power mode during periods of
inactivity.

This driver provides devicetree properties to program the device's behaviour
and a simple, tested and documented sysfs interface. The data sheet and more
information is available on Freescale's website.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

patch revision history
......................
v4 changes DT propery names, adds a missing interrupt source and removes
   the DT option to set interrupt line active high due to unsuccesful testing
v3 moves the driver from drivers/input/misc to drivers/misc
v2 corrects licensing and commit messages and adds appropriate recipients


 .../testing/sysfs-bus-i2c-devices-fsl-mma8653fc    |  39 +
 .../devicetree/bindings/misc/fsl,mma8653fc.txt     | 102 +++
 MAINTAINERS                                        |   5 +
 drivers/misc/Kconfig                               |  11 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/mma8653fc.c                           | 897 +++++++++++++++++++++
 6 files changed, 1055 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-fsl-mma8653fc
 create mode 100644 Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt
 create mode 100644 drivers/misc/mma8653fc.c

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-fsl-mma8653fc b/Documentation/ABI/testing/sysfs-bus-i2c-devices-fsl-mma8653fc
new file mode 100644
index 0000000..8172c27
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-fsl-mma8653fc
@@ -0,0 +1,39 @@
+What:		/sys/bus/i2c/drivers/mma8653fc/*/standby
+Date:		March 2015
+Contact:	Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
+Description:
+		Write 0 to this in order to turn on the device, and 1 to turn
+		it off. Read to see if it is turned on or off.
+
+
+What:		/sys/bus/i2c/drivers/mma8653fc/*/currentmode
+Date:		March 2015
+Contact:	Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
+Description:
+		Reading this provides the current state of the device, read
+		directly from a register. This can be "standby", "wake" or
+		"sleep".
+
+
+What:		/sys/bus/i2c/drivers/mma8653fc/*/position
+Date:		March 2015
+Contact:	Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
+Description:
+		Read only. Without interrupts enabled gets current position
+		values by reading. Poll "position" with interrupt conditions
+		set, to get notified; see Documentation/.../fsl,mma8653fc.txt
+
+		position file format:
+		"x y z [landscape/portrait status] [front/back status]"
+
+		x y z values:
+			in mg
+		landscape/portrait status char:
+			r	landscape right
+			d	portrait down
+			u	portrait up
+			l	landscape left
+		front/back status char:
+			f	front facing
+			b	back facing
+
diff --git a/Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt b/Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt
new file mode 100644
index 0000000..5015813
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt
@@ -0,0 +1,102 @@
+Freescale MMA8653FC 3-axis Accelerometer
+
+Required properties:
+- compatible
+	"fsl,mma8653fc"
+- reg
+	I2C address
+
+Optional properties:
+
+- interrupt-parent
+	a phandle for the interrupt controller (see
+	Documentation/devicetree/bindings/interrupt-controller/interrupts.txt)
+- interrupts
+	interrupt line to which the chip is connected (active low)
+- int1
+	set to use interrupt line 1, default is line 2
+	the interrupt sources can be routed to one of the two lines
+- ir-freefall-motion-x
+	activate freefall/motion interrupts on x axis
+- ir-freefall-motion-y
+	activate freefall/motion interrupts on y axis
+- ir-freefall-motion-z
+	activate freefall/motion interrupts on z axis
+- irq-threshold
+	0 < value < 8000: threshold for motion interrupts in mg
+- ir-landscape-portrait
+	activate landscape/portrait interrupts
+- ir-auto-wake
+	activate wake/sleep change interrupts
+- ir-data-ready:
+	activate data-ready interrupts
+	Interrupt events can be activated in any combination.
+- dynamic-range
+	2, 4, or 8: dynamic measurement range in g, default: 2
+	In ±2 g mode, sensitivity = 256 counts/g.
+	In ±4 g mode, sensitivity = 128 counts/g.
+	In ±8 g mode, sensitivity = 64 counts/g.
+- auto-wake-sleep
+	auto sleep mode (lower frequency)
+- motion-mode
+	use motion mode instead of freefall mode (trigger if >threshold).
+	per default an interrupt occurs if motion values fall below the
+	value set in "threshold" and therefore can detect free fall on the
+	vertical axis (depending on the position of the device).
+	Setting this values inverts the behaviour and an interrupt occurs
+	above the threshold value, so usually activate horizontal axis in
+	this case.
+
+- x-offset
+	0 < value < 500: calibration offset in mg
+	The offset correction values are used to realign the Zero-g position
+	of the X, Y, and Z-axis after the device is mounted on a board.
+	this value has an offset of 250 itself:
+	0 is -250mg, 250 is 0 mg, 500 is 250mg
+- y-offset
+	see x-offset
+- z-offset
+	see x-offset
+
+Example 1:
+for a device laying on flat ground to recognize acceleration over 100mg.
+x-axis is calibrated to +10mg. Adapt interrupt line to your device.
+
+mma8653fc@1d {
+		compatible = "fsl,mma8653fc";
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+		reg = <0x1d>;
+
+		dynamic-range = <2>;
+		motion-mode;
+		ir-freefall-motion-x;
+		ir-freefall-motion-y;
+		irq-threshold = <100>;
+		x-offset = <160>;
+};
+
+Example 2:
+for a device mounted on a wall with y being the vertical axis. This recognizes
+y-acceleration below 800mg, so free fall or changing the orientation of the
+device (y not being the vertical axis and having ~1000mg anymore).
+
+mma8653fc@1d {
+		compatible = "fsl,mma8653fc";
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+		reg = <0x1d>;
+
+		dynamic-range = <2>;
+		ir-freefall-motion-y;
+		irq-threshold = <800>;
+};
+
+Example 3:
+minimal example. lets users read current acceleration values. No polling
+is available.
+
+mma8653fc@1d {
+		compatible = "fsl,mma8653fc";
+		reg = <0x1d>;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0e1abe8..04c080d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4104,6 +4104,11 @@ S:	Maintained
 F:	include/linux/platform_data/video-imxfb.h
 F:	drivers/video/fbdev/imxfb.c
 
+FREESCALE MMA8653FC ACCELEROMETER I2C DRIVER
+M:	Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
+S:	Maintained
+F:	drivers/input/misc/mma8653fc.c
+
 FREESCALE QUAD SPI DRIVER
 M:	Han Xu <han.xu@freescale.com>
 L:	linux-mtd@lists.infradead.org
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 006242c..05ef1c1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -324,6 +324,17 @@ config ISL29020
 	  This driver can also be built as a module.  If so, the module
 	  will be called isl29020.
 
+config MMA8653FC
+	tristate "MMA8653FC - Freescale's 3-Axis, 10-bit Digital Accelerometer"
+	depends on I2C
+	default n
+	help
+	  Say Y here if you want to support Freescale's MMA8653FC Accelerometer
+	  through I2C interface.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mma8653fc.
+
 config SENSORS_TSL2550
 	tristate "Taos TSL2550 ambient light sensor"
 	depends on I2C && SYSFS
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7d5c4cd..044c8fc 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ICS932S401)	+= ics932s401.o
 obj-$(CONFIG_LKDTM)		+= lkdtm.o
 obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
 obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
+obj-$(CONFIG_MMA8653FC)		+= mma8653fc.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_SENSORS_BH1780)	+= bh1780gli.o
 obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
diff --git a/drivers/misc/mma8653fc.c b/drivers/misc/mma8653fc.c
new file mode 100644
index 0000000..f9b8cf9
--- /dev/null
+++ b/drivers/misc/mma8653fc.c
@@ -0,0 +1,897 @@
+/*
+ * mma8653fc.c - Support for Freescale MMA8653FC 3-axis 10-bit accelerometer
+ *
+ * Copyright (c) 2014 Theobroma Systems Design and Consulting GmbH
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/types.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+
+#define DRV_NAME	"mma8653fc"
+#define MMA8653FC_DEVICE_ID	0x5a
+
+#define MMA8653FC_STATUS	0x00
+
+#define ZYXOW_MASK	0x80
+#define ZYXDR		0x08
+
+#define MMA8653FC_WHO_AM_I	0x0d
+
+#define MMA8653FC_SYSMOD	0x0b
+#define STATE_STANDBY	0x00
+#define STATE_WAKE	0x01
+#define STATE_SLEEP	0x02
+
+#define MMA8450_STATUS_ZXYDR	0x08
+
+/*
+ * 10 bit output data registers
+ * MSB: 7:0 bits 9:2 of data word
+ * LSB: 7:6 bits 1:0 of data word
+ */
+#define MMA8653FC_OUT_X_MSB	0x01
+#define MMA8653FC_OUT_X_LSB	0x02
+#define MMA8653FC_OUT_Y_MSB	0x03
+#define MMA8653FC_OUT_Y_LSB	0x04
+#define MMA8653FC_OUT_Z_MSB	0x05
+#define MMA8653FC_OUT_Z_LSB	0x06
+
+/*
+ * Portrait/Landscape Status
+ */
+#define MMA8653FC_PL_STATUS	0x10
+
+#define NEWLP		0x80
+#define LAPO_HIGH	0x04
+#define LAPO_LOW	0x02
+#define BAFRO		0x01
+
+/*
+ * Portrait/Landscape Configuration
+ */
+#define MMA8653FC_PL_CFG	0x11
+
+#define PL_EN		(1 << 6)
+
+/*
+ * Data calibration registers
+ */
+#define MMA8653FC_OFF_X		0x2f
+#define MMA8653FC_OFF_Y		0x30
+#define MMA8653FC_OFF_Z		0x31
+
+/* 0 to 500 for dts, but -250 to 250 in mg */
+#define DEFAULT_OFF	250
+
+/*
+ * bits 1:0 dynamic range
+ * 00 +/- 2g
+ * 01 +/- 4g
+ * 10 +/- 8g
+ *
+ * HPF_Out bit 4 - data high pass or low pass filtered
+ */
+#define MMA8653FC_XYZ_DATA_CFG	0x0e
+
+#define RANGE_MASK	0x03
+#define RANGE2G		0x00
+#define RANGE4G		0x01
+#define RANGE8G		0x02
+/* values for calculation */
+#define SHIFT_2G	8
+#define INCR_2G		128
+#define SHIFT_4G	7
+#define INCR_4G		64
+#define SHIFT_8G	6
+#define INCR_8G		32
+#define DYN_RANGE_2G	2
+#define DYN_RANGE_4G	4
+#define DYN_RANGE_8G	8
+
+/*
+ * System Control Reg 1
+ */
+#define MMA8653FC_CTRL_REG1	0x2a
+
+#define ACTIVE_BIT	(1 << 0)
+#define ODR_MASK	0x38
+#define ODR_DEFAULT	0x20 /* 50 Hz */
+#define ASLP_RATE_MASK	0xc0
+#define ASLP_RATE_DEFAULT 0x80 /* 6.25 Hz */
+
+/*
+ * Sys Control Reg 2
+ *
+ * auto-sleep enable, software reset
+ */
+#define MMA8653FC_CTRL_REG2	0x2b
+
+#define SLPE		(1 << 2)
+#define SELFTEST	(1 << 7)
+#define SOFT_RESET	(1 << 6)
+
+/*
+ * Interrupt Source
+ */
+#define MMA8653FC_INT_SOURCE	0x0c
+
+#define SRC_ASLP	(1 << 7)
+#define SRC_LNDPRT	(1 << 4)
+#define SRC_FF_MT	(1 << 2)
+#define SRC_DRDY	(1 << 0)
+
+/*
+ * Interrupt Control Register
+ *
+ * default: active low
+ * default push-pull, not open-drain
+ */
+#define MMA8653FC_CTRL_REG3	0x2c
+
+#define WAKE_LNDPRT	(1 << 5)
+#define WAKE_FF_MT	(1 << 3)
+#define IPOL		(1 << 1)
+#define PP_OD		(1 << 0)
+
+/*
+ * Interrupt Enable Register
+ */
+#define MMA8653FC_CTRL_REG4	0x2d
+
+#define INT_EN_ASLP	(1 << 7)
+#define INT_EN_LNDPRT	(1 << 4)
+#define INT_EN_FF_MT	(1 << 2)
+#define INT_EN_DRDY	(1 << 0)
+
+/*
+ * Interrupt Configuration Register
+ * bit value 0 ... INT2 (default)
+ * bit value 1 ... INT1
+ */
+#define MMA8653FC_CTRL_REG5	0x2e
+
+#define INT_CFG_ASLP	(1 << 7)
+#define INT_CFG_LNDPRT	(1 << 4)
+#define INT_CFG_FF_MT	(1 << 2)
+#define INT_CFG_DRDY	(1 << 0)
+
+/*
+ * Freefall / Motion Configuration Register
+ *
+ * Event Latch enable/disable, motion or freefall mode
+ * and event flag enable per axis
+ */
+#define MMA8653FC_FF_MT_CFG	0x15
+
+#define FF_MT_CFG_ELE	(1 << 7)
+#define FF_MT_CFG_OAE	(1 << 6)
+#define FF_MT_CFG_ZEFE	(1 << 5)
+#define FF_MT_CFG_YEFE	(1 << 4)
+#define FF_MT_CFG_XEFE	(1 << 3)
+
+/*
+ * Freefall / Motion Source Register
+ */
+#define MMA8653FC_FF_MT_SRC	0x16
+
+/*
+ * Freefall / Motion Threshold Register
+ *
+ * define motion threshold
+ * 0.063 g/LSB, 127 counts(0x7f) (7 bit from LSB)
+ * range: 0.063g - 8g
+ */
+#define MMA8653FC_FF_MT_THS	0x17
+
+struct axis_triple {
+	s16 x;
+	s16 y;
+	s16 z;
+};
+
+struct mma8653fc_pdata {
+	s8 x_axis_offset;
+	s8 y_axis_offset;
+	s8 z_axis_offset;
+	bool auto_wake_sleep;
+	u32 range;
+	bool int1;
+	bool motion_mode;
+	u8 freefall_motion_thr;
+	bool int_src_data_ready;
+	bool int_src_ff_mt_x;
+	bool int_src_ff_mt_y;
+	bool int_src_ff_mt_z;
+	bool int_src_lndprt;
+	bool int_src_aslp;
+};
+
+struct mma8653fc {
+	struct i2c_client *client;
+	struct mutex mutex;
+	struct mma8653fc_pdata pdata;
+	struct axis_triple saved;
+	char orientation;
+	char bafro;
+	bool standby;
+	int irq;
+	unsigned int_mask;
+	u8 (*read)(struct mma8653fc *, unsigned char);
+	int (*write)(struct mma8653fc *, unsigned char, unsigned char);
+};
+
+/* defaults */
+static const struct mma8653fc_pdata mma8653fc_default_init = {
+	.range = 2,
+	.x_axis_offset = DEFAULT_OFF,
+	.y_axis_offset = DEFAULT_OFF,
+	.z_axis_offset = DEFAULT_OFF,
+	.auto_wake_sleep = false,
+	.int1 = false,
+	.motion_mode = false,
+	.freefall_motion_thr = 5,
+	.int_src_data_ready = false,
+	.int_src_ff_mt_x = false,
+	.int_src_ff_mt_y = false,
+	.int_src_ff_mt_z = false,
+	.int_src_lndprt = false,
+	.int_src_aslp = false,
+};
+
+static void mma8653fc_get_triple(struct mma8653fc *mma)
+{
+	u8 buf[6];
+	u8 status;
+
+	buf[0] = 0;
+
+	status = mma->read(mma, MMA8653FC_STATUS);
+	if (status & ZYXOW_MASK)
+		dev_dbg(&mma->client->dev, "previous read not completed\n");
+
+	buf[0] = mma->read(mma, MMA8653FC_OUT_X_MSB);
+	buf[1] = mma->read(mma, MMA8653FC_OUT_X_LSB);
+	buf[2] = mma->read(mma, MMA8653FC_OUT_Y_MSB);
+	buf[3] = mma->read(mma, MMA8653FC_OUT_Y_LSB);
+	buf[4] = mma->read(mma, MMA8653FC_OUT_Z_MSB);
+	buf[5] = mma->read(mma, MMA8653FC_OUT_Z_LSB);
+
+	mutex_lock(&mma->mutex);
+	/* move from registers to s16 */
+	mma->saved.x = (buf[1] | (buf[0] << 8)) >> 6;
+	mma->saved.y = (buf[3] | (buf[2] << 8)) >> 6;
+	mma->saved.z = (buf[5] | (buf[4] << 8)) >> 6;
+	mma->saved.x = sign_extend32(mma->saved.x, 9);
+	mma->saved.y = sign_extend32(mma->saved.y, 9);
+	mma->saved.z = sign_extend32(mma->saved.z, 9);
+
+	/* calc g, see data sheet and application note */
+	switch (mma->pdata.range) {
+	case DYN_RANGE_2G:
+		mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
+					    INCR_2G) >> SHIFT_2G);
+		mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
+					   INCR_2G) >> SHIFT_2G);
+		mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
+					   INCR_2G) >> SHIFT_2G);
+		break;
+	case DYN_RANGE_4G:
+		mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
+					   INCR_4G) >> SHIFT_4G);
+		mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
+					   INCR_4G) >> SHIFT_4G);
+		mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
+					   INCR_4G) >> SHIFT_4G);
+		break;
+	case DYN_RANGE_8G:
+		mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
+					   INCR_8G) >> SHIFT_8G);
+		mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
+					   INCR_8G) >> SHIFT_8G);
+		mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
+					   INCR_8G) >> SHIFT_8G);
+		break;
+	default:
+		dev_err(&mma->client->dev, "internal data corrupt\n");
+	}
+	mutex_unlock(&mma->mutex);
+}
+
+static void mma8653fc_get_orientation(struct mma8653fc *mma, u8 byte)
+{
+	if ((byte & LAPO_HIGH) && !(LAPO_LOW))
+		mma->orientation = 'r'; /* landscape right */
+	if (!(byte & LAPO_HIGH) && (byte & LAPO_LOW))
+		mma->orientation = 'd'; /* portrait down */
+	if (!(byte & LAPO_HIGH) && !(byte & LAPO_LOW))
+		mma->orientation = 'u'; /* portrait up */
+	if ((byte & LAPO_HIGH) && (byte & LAPO_LOW))
+		mma->orientation = 'l'; /* landscape left */
+
+	if (byte & BAFRO)
+		mma->bafro = 'b'; /* back facing */
+	else
+		mma->bafro = 'f'; /* front facing */
+}
+
+static irqreturn_t mma8653fc_irq(int irq, void *handle)
+{
+	struct mma8653fc *mma = handle;
+	u8 int_src;
+	u8 byte;
+
+	int_src = mma->read(mma, MMA8653FC_INT_SOURCE);
+	if (int_src & SRC_DRDY)
+		/* data ready handle */
+	if (int_src & SRC_FF_MT) {
+		/* freefall/motion change handle */
+		dev_dbg(&mma->client->dev,
+			"freefall or motion change\n");
+		byte = mma->read(mma, MMA8653FC_FF_MT_SRC);
+	}
+	if (int_src & SRC_LNDPRT) {
+		/* landscape/portrait change handle */
+		dev_dbg(&mma->client->dev,
+			"landscape / portrait change\n");
+		byte = mma->read(mma, MMA8653FC_PL_STATUS);
+		mma8653fc_get_orientation(mma, byte);
+	}
+	if (int_src & SRC_ASLP)
+		/* autosleep change handle */
+	mma8653fc_get_triple(mma);
+
+	sysfs_notify(&mma->client->dev.kobj, NULL, "position");
+
+	return IRQ_HANDLED;
+}
+
+static void __mma8653fc_enable(struct mma8653fc *mma)
+{
+	u8 byte;
+
+	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
+	mma->write(mma, MMA8653FC_CTRL_REG1, byte | ACTIVE_BIT);
+}
+
+static void __mma8653fc_disable(struct mma8653fc *mma)
+{
+	u8 byte;
+
+	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
+	mma->write(mma, MMA8653FC_CTRL_REG1, byte & ~ACTIVE_BIT);
+}
+
+static ssize_t mma8653fc_standby_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n", mma->standby);
+}
+
+static ssize_t mma8653fc_standby_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+	unsigned int val;
+	int error;
+
+	error = kstrtouint(buf, 10, &val);
+	if (error)
+		return error;
+
+	mutex_lock(&mma->mutex);
+
+	if (val) {
+		if (!mma->standby)
+			__mma8653fc_disable(mma);
+	} else {
+		if (mma->standby)
+			__mma8653fc_enable(mma);
+	}
+
+	mma->standby = !!val;
+
+	mutex_unlock(&mma->mutex);
+
+	return count;
+}
+
+static DEVICE_ATTR(standby, 0664, mma8653fc_standby_show,
+				  mma8653fc_standby_store);
+
+static ssize_t mma8653fc_currentmode_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+	ssize_t count;
+	u8 byte;
+
+	byte = mma->read(mma, MMA8653FC_SYSMOD);
+	if (byte < 0)
+		return byte;
+
+	switch (byte) {
+	case STATE_STANDBY:
+		count = scnprintf(buf, PAGE_SIZE, "standby\n");
+		break;
+	case STATE_WAKE:
+		count = scnprintf(buf, PAGE_SIZE, "wake\n");
+		break;
+	case STATE_SLEEP:
+		count = scnprintf(buf, PAGE_SIZE, "sleep\n");
+		break;
+	default:
+		count = scnprintf(buf, PAGE_SIZE, "unknown\n");
+	}
+	return count;
+}
+static DEVICE_ATTR(currentmode, S_IRUGO, mma8653fc_currentmode_show, NULL);
+
+static ssize_t mma8653fc_position_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+	ssize_t count;
+	u8 byte;
+
+	if (!mma->irq) {
+		byte = mma->read(mma, MMA8653FC_PL_STATUS);
+		if (byte & NEWLP)
+			mma8653fc_get_orientation(mma, byte);
+
+		byte = mma->read(mma, MMA8653FC_STATUS);
+		if (byte & ZYXDR)
+			mma8653fc_get_triple(mma);
+
+		msleep(20);
+		dev_dbg(&client->dev, "data polled\n");
+	}
+	mutex_lock(&mma->mutex);
+	count = scnprintf(buf, PAGE_SIZE, "%d %d %d %c %c\n",
+			mma->saved.x, mma->saved.y, mma->saved.z,
+			mma->orientation, mma->bafro);
+	mutex_unlock(&mma->mutex);
+
+	return count;
+}
+static DEVICE_ATTR(position, S_IRUGO, mma8653fc_position_show, NULL);
+
+static struct attribute *mma8653fc_attributes[] = {
+	&dev_attr_position.attr,
+	&dev_attr_standby.attr,
+	&dev_attr_currentmode.attr,
+	NULL,
+};
+
+static const struct attribute_group mma8653fc_attr_group = {
+	.attrs = mma8653fc_attributes,
+};
+
+static u8 mma8653fc_read(struct mma8653fc *mma, unsigned char reg)
+{
+	u8 val;
+
+	val = i2c_smbus_read_byte_data(mma->client, reg);
+	if (val < 0) {
+		dev_err(&mma->client->dev,
+			"failed to read %x register\n", reg);
+	}
+	return val;
+}
+
+static int mma8653fc_write(struct mma8653fc *mma, unsigned char reg,
+			   unsigned char val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(mma->client, reg, val);
+	if (ret) {
+		dev_err(&mma->client->dev,
+			"failed to write %x register\n", reg);
+	}
+	return ret;
+}
+
+static const struct of_device_id mma8653fc_dt_ids[] = {
+	{ .compatible = "fsl,mma8653fc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mma8653fc_dt_ids);
+
+static const struct mma8653fc_pdata *mma8653fc_probe_dt(struct device *dev)
+{
+	struct mma8653fc_pdata *pdata;
+	struct device_node *node = dev->of_node;
+	const struct of_device_id *match;
+	u32 testu32;
+	s32 tests32;
+
+	if (!node) {
+		dev_err(dev, "no associated DT data\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	match = of_match_device(mma8653fc_dt_ids, dev);
+	if (!match) {
+		dev_err(dev, "unknown device model\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	*pdata = mma8653fc_default_init;
+
+	/* overwrite from dts */
+	testu32 = pdata->x_axis_offset;
+	tests32 = 0;
+	of_property_read_u32(node, "x-offset", &testu32);
+	tests32 = testu32 - DEFAULT_OFF;
+	if ((tests32) && (tests32 <= DEFAULT_OFF) &&
+	    (tests32 >= -DEFAULT_OFF)) {
+		dev_info(dev, "use %dmg offset on X axis\n", tests32);
+		/* calc register value, resolution: 1.96mg */
+		pdata->x_axis_offset = (s8) (tests32 / 2);
+	}
+	testu32 = pdata->y_axis_offset;
+	tests32 = 0;
+	of_property_read_u32(node, "y-offset", &testu32);
+	tests32 = testu32 - DEFAULT_OFF;
+	if ((tests32) && (tests32 <= DEFAULT_OFF) &&
+	    (tests32 >= -DEFAULT_OFF)) {
+		dev_info(dev, "use %dmg offset on Y axis\n", tests32);
+		pdata->y_axis_offset = (s8) (tests32 / 2);
+	}
+	testu32 = pdata->z_axis_offset;
+	tests32 = 0;
+	of_property_read_u32(node, "z-offset", &testu32);
+	tests32 = testu32 - DEFAULT_OFF;
+	if ((tests32) && (tests32 <= DEFAULT_OFF) &&
+	    (tests32 >= -DEFAULT_OFF)) {
+		dev_info(dev, "use %dmg offset on Z axis\n", tests32);
+		pdata->z_axis_offset = (s8) (tests32 / 2);
+	}
+
+	testu32 = 0;
+	of_property_read_u32(node, "dynamic-range", &testu32);
+	if ((testu32) && (testu32 != 2) && (testu32 != 4) && (testu32 != 8)) {
+		dev_warn(dev, "wrong value for full scale range in dtb\n");
+	} else {
+		if (testu32)
+			pdata->range = testu32;
+	}
+
+	if (of_property_read_bool(node, "auto-wake-sleep"))
+		pdata->auto_wake_sleep = true;
+
+	if (of_property_read_bool(node, "int1"))
+		pdata->int1 = true;
+
+	if (of_property_read_bool(node, "motion-mode"))
+		pdata->motion_mode = true;
+
+	if (of_property_read_bool(node, "ir-data-ready"))
+		pdata->int_src_data_ready = true;
+
+	if (of_property_read_bool(node, "ir-freefall-motion-x"))
+		pdata->int_src_ff_mt_x = true;
+
+	if (of_property_read_bool(node, "ir-freefall-motion-y"))
+		pdata->int_src_ff_mt_y = true;
+
+	if (of_property_read_bool(node, "ir-freefall-motion-z"))
+		pdata->int_src_ff_mt_z = true;
+
+	if (of_property_read_bool(node, "ir-auto-wake"))
+		pdata->int_src_aslp = true;
+
+	if (of_property_read_bool(node, "ir-landscape-portrait"))
+		pdata->int_src_lndprt = true;
+
+	testu32 = 0;
+	of_property_read_u32(node, "irq-threshold", &testu32);
+	/* always uses maximum range +/- 8000g, resolution 63mg */
+	if ((testu32 <= 8000) && (testu32 > 0)) {
+		dev_dbg(dev, "use freefall / motion threshold %dmg\n",
+			 testu32);
+		/* calculate register value from mg */
+		pdata->freefall_motion_thr = (testu32 / 63) + 1;
+	}
+
+	return pdata;
+}
+static int mma8653fc_i2c_probe(struct i2c_client *client,
+				       const struct i2c_device_id *id)
+{
+	struct mma8653fc *mma;
+	const struct mma8653fc_pdata *pdata;
+	int err;
+	u8 byte;
+
+	err = i2c_check_functionality(client->adapter,
+			I2C_FUNC_SMBUS_BYTE_DATA);
+	if (!err) {
+		dev_err(&client->dev, "SMBUS Byte Data not Supported\n");
+		return -EIO;
+	}
+
+	mma = kzalloc(sizeof(*mma), GFP_KERNEL);
+	if (!mma) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	pdata = dev_get_platdata(&client->dev);
+	if (!pdata) {
+		pdata = mma8653fc_probe_dt(&client->dev);
+		if (IS_ERR(pdata)) {
+			err = PTR_ERR(pdata);
+			pr_err("pdata from DT failed\n");
+			goto err_free_mem;
+		}
+	}
+	mma->pdata = *pdata;
+	pdata = &mma->pdata;
+	mma->client = client;
+	mma->read = &mma8653fc_read;
+	mma->write = &mma8653fc_write;
+
+	mutex_init(&mma->mutex);
+
+	i2c_set_clientdata(client, mma);
+
+	err = sysfs_create_group(&client->dev.kobj, &mma8653fc_attr_group);
+	if (err)
+		goto err_free_mem;
+
+	mma->irq = irq_of_parse_and_map(client->dev.of_node, 0);
+	if (!mma->irq) {
+		dev_err(&client->dev, "Unable to parse or map IRQ\n");
+		goto no_irq;
+	}
+
+	err = irq_set_irq_type(mma->irq, IRQ_TYPE_EDGE_FALLING);
+	if (err) {
+		dev_err(&client->dev, "set_irq_type failed\n");
+		goto err_free_mem;
+	}
+
+	err = request_threaded_irq(mma->irq, NULL, mma8653fc_irq, IRQF_ONESHOT,
+				   dev_name(&mma->client->dev), mma);
+	if (err) {
+		dev_err(&client->dev, "irq %d busy?\n", mma->irq);
+		goto err_free_mem;
+	}
+
+	if (mma->write(mma, MMA8653FC_CTRL_REG2, SOFT_RESET))
+		goto err_free_irq;
+
+	__mma8653fc_disable(mma);
+	mma->standby = true;
+
+	/* enable desired interrupts */
+	mma->orientation = '\0';
+	mma->bafro = '\0';
+	byte = 0;
+	if (pdata->int_src_data_ready) {
+		byte |= INT_EN_DRDY;
+		dev_dbg(&client->dev, "DATA READY interrupt source enabled\n");
+	}
+	if (pdata->int_src_ff_mt_x || pdata->int_src_ff_mt_y ||
+	    pdata->int_src_ff_mt_z) {
+		byte |= INT_EN_FF_MT;
+		dev_dbg(&client->dev, "FF MT interrupt source enabled\n");
+	}
+	if (pdata->int_src_lndprt) {
+		if (mma->write(mma, MMA8653FC_PL_CFG, PL_EN))
+			goto err_free_irq;
+		byte |= INT_EN_LNDPRT;
+		dev_dbg(&client->dev, "LNDPRT interrupt source enabled\n");
+	}
+	if (pdata->int_src_aslp) {
+		byte |= INT_EN_ASLP;
+		dev_dbg(&client->dev, "ASLP interrupt source enabled\n");
+	}
+	if (mma->write(mma, MMA8653FC_CTRL_REG4, byte))
+		goto err_free_irq;
+
+	/* force everything to line 1 */
+	if (pdata->int1) {
+		if (mma->write(mma, MMA8653FC_CTRL_REG5,
+				 (INT_CFG_ASLP | INT_CFG_LNDPRT |
+				 INT_CFG_FF_MT | INT_CFG_DRDY)))
+			goto err_free_irq;
+		dev_dbg(&client->dev, "using interrupt line 1\n");
+	}
+no_irq:
+	/* range mode */
+	byte = mma->read(mma, MMA8653FC_XYZ_DATA_CFG);
+	byte &= ~RANGE_MASK;
+	switch (pdata->range) {
+	case DYN_RANGE_2G:
+		byte |= RANGE2G;
+		dev_dbg(&client->dev, "use 2g range\n");
+		break;
+	case DYN_RANGE_4G:
+		byte |= RANGE4G;
+		dev_dbg(&client->dev, "use 4g range\n");
+		break;
+	case DYN_RANGE_8G:
+		byte |= RANGE8G;
+		dev_dbg(&client->dev, "use 8g range\n");
+		break;
+	default:
+		dev_err(&client->dev, "wrong range mode value\n");
+		goto err_free_irq;
+	}
+	if (mma->write(mma, MMA8653FC_XYZ_DATA_CFG, byte))
+		goto err_free_irq;
+
+	/* data calibration offsets */
+	if (pdata->x_axis_offset) {
+		if (mma->write(mma, MMA8653FC_OFF_X, pdata->x_axis_offset))
+			goto err_free_irq;
+	}
+	if (pdata->y_axis_offset) {
+		if (mma->write(mma, MMA8653FC_OFF_Y, pdata->y_axis_offset))
+			goto err_free_irq;
+	}
+	if (pdata->z_axis_offset) {
+		if (mma->write(mma, MMA8653FC_OFF_Z, pdata->z_axis_offset))
+			goto err_free_irq;
+	}
+
+	/* if autosleep, wake on both landscape and motion changes */
+	if (pdata->auto_wake_sleep) {
+		byte = 0;
+		byte |= WAKE_LNDPRT;
+		byte |= WAKE_FF_MT;
+		if (mma->write(mma, MMA8653FC_CTRL_REG3, byte))
+			goto err_free_irq;
+		if (mma->write(mma, MMA8653FC_CTRL_REG2, SLPE))
+			goto err_free_irq;
+		dev_dbg(&client->dev, "auto sleep enabled\n");
+	}
+
+	/* data rates */
+	byte = 0;
+	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
+	if (byte < 0)
+		goto err_free_irq;
+	byte &= ~ODR_MASK;
+	byte |= ODR_DEFAULT;
+	byte &= ~ASLP_RATE_MASK;
+	byte |= ASLP_RATE_DEFAULT;
+	if (mma->write(mma, MMA8653FC_CTRL_REG1, byte))
+		goto err_free_irq;
+
+	/* freefall / motion config */
+	byte = 0;
+	if (pdata->motion_mode) {
+		byte |= FF_MT_CFG_OAE;
+		dev_dbg(&client->dev, "detect motion instead of freefall\n");
+	}
+	byte |= FF_MT_CFG_ELE;
+	if (pdata->int_src_ff_mt_x)
+		byte |= FF_MT_CFG_XEFE;
+	if (pdata->int_src_ff_mt_y)
+		byte |= FF_MT_CFG_YEFE;
+	if (pdata->int_src_ff_mt_z)
+		byte |= FF_MT_CFG_ZEFE;
+	if (mma->write(mma, MMA8653FC_FF_MT_CFG, byte))
+		goto err_free_irq;
+
+	if (pdata->freefall_motion_thr) {
+		if (mma->write(mma, MMA8653FC_FF_MT_THS,
+				 pdata->freefall_motion_thr))
+			goto err_free_irq;
+		/* calculate back to mg */
+		dev_dbg(&client->dev, "threshold set to %dmg\n",
+			 (63 * pdata->freefall_motion_thr) - 1);
+	}
+
+	byte = mma->read(mma, MMA8653FC_WHO_AM_I);
+	if (byte != MMA8653FC_DEVICE_ID) {
+		dev_err(&client->dev, "wrong device for driver\n");
+		goto err_free_irq;
+	}
+	dev_info(&client->dev,
+		 "accelerometer driver loaded. device id %x\n", byte);
+
+	return 0;
+
+ err_free_irq:
+	if (mma->irq)
+		free_irq(mma->irq, mma);
+ err_free_mem:
+	kfree(mma);
+ err_out:
+	return err;
+}
+
+static int mma8653fc_remove(struct i2c_client *client)
+{
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	free_irq(mma->irq, mma);
+	dev_dbg(&client->dev, "unregistered accelerometer\n");
+	kfree(mma);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mma8653fc_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	__mma8653fc_disable(mma);
+
+	return 0;
+}
+
+static int mma8653fc_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	__mma8653fc_enable(mma);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mma8653fc_pm_ops, mma8653fc_suspend, mma8653fc_resume);
+#define MMA8653FC_PM_OPS (&mma8653fc_pm_ops)
+#else
+#define MMA8653FC_PM_OPS NULL
+#endif
+
+static const struct i2c_device_id mma8653fc_id[] = {
+	{ DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma8653fc_id);
+
+static struct i2c_driver mma8653fc_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = mma8653fc_dt_ids,
+		.pm = MMA8653FC_PM_OPS,
+	},
+	.probe    = mma8653fc_i2c_probe,
+	.remove   = mma8653fc_remove,
+	.id_table = mma8653fc_id,
+};
+
+module_i2c_driver(mma8653fc_driver);
+
+MODULE_AUTHOR("Martin Kepplinger <martin.kepplinger@theobroma-systems.com");
+MODULE_DESCRIPTION("Freescale's MMA8653FC Three-Axis Accelerometer I2C Driver");
+MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH v2] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Martin Kepplinger @ 2015-03-20 11:26 UTC (permalink / raw)
  To: Bastien Nocera, Martin Kepplinger
  Cc: Alexander Stein, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, dmitry.torokhov, akpm, gregkh, linux-api,
	devicetree, linux-input, linux-kernel, Christoph Muellner
In-Reply-To: <1426760577.6764.14.camel@hadess.net>

Am 2015-03-19 um 11:22 schrieb Bastien Nocera:
> On Wed, 2015-03-18 at 19:28 +0100, Martin Kepplinger wrote:
>> Am 2015-03-18 um 19:05 schrieb Bastien Nocera:
>>> On Wed, 2015-03-18 at 19:02 +0100, Martin Kepplinger wrote:
>>>> Am 2015-03-18 um 17:59 schrieb Bastien Nocera:
>>>>> On Wed, 2015-03-18 at 17:42 +0100, Martin Kepplinger wrote:
>>>>>>
>>>>> <snip>
>>>>>> It could have gone to drivers/iio/accel if it would use an 
>>>>>> iio   interface, which would make more sense, you are right, 
>>>>>> but I 
>>>>>> simply  don't have the time to merge it in to iio.
>>>>>>
>>>>>> It doesn't use an input interface either but I don't see a 
>>>>>> good   place for an accelerometer that uses sysfs only.
>>>>>>
>>>>>> It works well, is a relatively recent chip and a clean 
>>>>>> dirver.  But  this is all I can provide.
>>>>>
>>>>> As a person who works on the user-space interaction of those 
>>>>> with   desktops [1]: Urgh.
>>>>>
>>>>> I already have 3 (probably 4) types of accelerometers to 
>>>>> contend  with,  I'm not fond of adding yet another type.
>>>>>
>>>>> Is there any way to get this hardware working outside the SoCs 
>>>>> it's  designed for (say, a device with I2C like a Raspberry 
>>>>> Pi),  so that a  kind soul could handle getting this using the 
>>>>> right  interfaces?
>>>>>
>>>>
>>>> It works on basically any SoC and is in no way limited in this  
>>>> regard. Sure, userspace has to expicitely support it and I hear 
>>>> you.  Using the iio interface would make more sense. I can only 
>>>> say I'd  love to have the time to move this driver over. I'm 
>>>> very sorry.
>>>
>>> How can we get the hardware for somebody to use on their own  
>>> laptops/embedded boards to implement this driver?
>>>
>>
>> It's connected over I2C. If the included documentation is not clear 
>> please tell me what exacly. Thanks!
> 
> 
> I'll ask the question a different way: can you please give the address 
> of a shop where that hardware is available?
> 

there is
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=RDMMA865x&lang_cd=
and http://linux-sunxi.org/Inet_K970 for example. I think I saw Android
devices with it too, and I would guess it would be used more often if it
were in linux.

Please refer to v4 of the patch for different questions. thanks!

^ permalink raw reply

* Re: [PATCH v4] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Varka Bhadram @ 2015-03-20 11:31 UTC (permalink / raw)
  To: Martin Kepplinger, mark.rutland, robh+dt, Pawel.Moll,
	ijc+devicetree, galak, dmitry.torokhov, alexander.stein
  Cc: hadess, akpm, gregkh, linux-api, devicetree, linux-input,
	linux-kernel, Martin Kepplinger, Christoph Muellner
In-Reply-To: <1426850431-31550-1-git-send-email-martink@posteo.de>

On 03/20/2015 04:50 PM, Martin Kepplinger wrote:
> From: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>
> The MMA8653FC is a low-power, three-axis, capacitive micromachined
> accelerometer with 10 bits of resolution with flexible user-programmable
> options.
>
> Embedded interrupt functions enable overall power savings, by relieving the
> host processor from continuously polling data, for example using the poll()
> system call.
>
> The device can be configured to generate wake-up interrupt signals from any
> combination of the configurable embedded functions, enabling the MMA8653FC
> to monitor events while remaining in a low-power mode during periods of
> inactivity.
>
> This driver provides devicetree properties to program the device's behaviour
> and a simple, tested and documented sysfs interface. The data sheet and more
> information is available on Freescale's website.
>
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>
(...)

> +static int mma8653fc_i2c_probe(struct i2c_client *client,
> +				       const struct i2c_device_id *id)
> +{
> +	struct mma8653fc *mma;
> +	const struct mma8653fc_pdata *pdata;
> +	int err;
> +	u8 byte;
> +
> +	err = i2c_check_functionality(client->adapter,
> +			I2C_FUNC_SMBUS_BYTE_DATA);
> +	if (!err) {
> +		dev_err(&client->dev, "SMBUS Byte Data not Supported\n");
> +		return -EIO;
> +	}
> +
> +	mma = kzalloc(sizeof(*mma), GFP_KERNEL);

Why not devm_* API..?

> +	if (!mma) {
> +		err = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	pdata = dev_get_platdata(&client->dev);
> +	if (!pdata) {
> +		pdata = mma8653fc_probe_dt(&client->dev);
> +		if (IS_ERR(pdata)) {
> +			err = PTR_ERR(pdata);
> +			pr_err("pdata from DT failed\n");

Use dev_err() instead of pr_err()..

> +			goto err_free_mem;
> +		}
> +	}
> +	mma->pdata = *pdata;
> +	pdata = &mma->pdata;
> +	mma->client = client;
> +	mma->read = &mma8653fc_read;
> +	mma->write = &mma8653fc_write;
> +
> +	mutex_init(&mma->mutex);
> +
> +	i2c_set_clientdata(client, mma);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &mma8653fc_attr_group);
> +	if (err)
> +		goto err_free_mem;
> +
> +	mma->irq = irq_of_parse_and_map(client->dev.of_node, 0);
> +	if (!mma->irq) {
> +		dev_err(&client->dev, "Unable to parse or map IRQ\n");
> +		goto no_irq;
> +	}
> +
> +	err = irq_set_irq_type(mma->irq, IRQ_TYPE_EDGE_FALLING);
> +	if (err) {
> +		dev_err(&client->dev, "set_irq_type failed\n");
> +		goto err_free_mem;
> +	}
> +
> +	err = request_threaded_irq(mma->irq, NULL, mma8653fc_irq, IRQF_ONESHOT,
> +				   dev_name(&mma->client->dev), mma);

devm_* API..?

> +	if (err) {
> +		dev_err(&client->dev, "irq %d busy?\n", mma->irq);
> +		goto err_free_mem;
> +	}
> +
> +	if (mma->write(mma, MMA8653FC_CTRL_REG2, SOFT_RESET))
> +		goto err_free_irq;
> +
> +	__mma8653fc_disable(mma);
> +	mma->standby = true;
> +
> +	/* enable desired interrupts */
> +	mma->orientation = '\0';
> +	mma->bafro = '\0';
> +	byte = 0;
> +	if (pdata->int_src_data_ready) {
> +		byte |= INT_EN_DRDY;
> +		dev_dbg(&client->dev, "DATA READY interrupt source enabled\n");
> +	}
> +	if (pdata->int_src_ff_mt_x || pdata->int_src_ff_mt_y ||
> +	    pdata->int_src_ff_mt_z) {
> +		byte |= INT_EN_FF_MT;
> +		dev_dbg(&client->dev, "FF MT interrupt source enabled\n");
> +	}
> +	if (pdata->int_src_lndprt) {
> +		if (mma->write(mma, MMA8653FC_PL_CFG, PL_EN))
> +			goto err_free_irq;
> +		byte |= INT_EN_LNDPRT;
> +		dev_dbg(&client->dev, "LNDPRT interrupt source enabled\n");
> +	}
> +	if (pdata->int_src_aslp) {
> +		byte |= INT_EN_ASLP;
> +		dev_dbg(&client->dev, "ASLP interrupt source enabled\n");
> +	}
> +	if (mma->write(mma, MMA8653FC_CTRL_REG4, byte))
> +		goto err_free_irq;
> +
> +	/* force everything to line 1 */
> +	if (pdata->int1) {
> +		if (mma->write(mma, MMA8653FC_CTRL_REG5,
> +				 (INT_CFG_ASLP | INT_CFG_LNDPRT |
> +				 INT_CFG_FF_MT | INT_CFG_DRDY)))
> +			goto err_free_irq;
> +		dev_dbg(&client->dev, "using interrupt line 1\n");
> +	}
> +no_irq:
> +	/* range mode */
> +	byte = mma->read(mma, MMA8653FC_XYZ_DATA_CFG);
> +	byte &= ~RANGE_MASK;
> +	switch (pdata->range) {
> +	case DYN_RANGE_2G:
> +		byte |= RANGE2G;
> +		dev_dbg(&client->dev, "use 2g range\n");
> +		break;
> +	case DYN_RANGE_4G:
> +		byte |= RANGE4G;
> +		dev_dbg(&client->dev, "use 4g range\n");
> +		break;
> +	case DYN_RANGE_8G:
> +		byte |= RANGE8G;
> +		dev_dbg(&client->dev, "use 8g range\n");
> +		break;
> +	default:
> +		dev_err(&client->dev, "wrong range mode value\n");
> +		goto err_free_irq;
> +	}
> +	if (mma->write(mma, MMA8653FC_XYZ_DATA_CFG, byte))
> +		goto err_free_irq;
> +
> +	/* data calibration offsets */
> +	if (pdata->x_axis_offset) {
> +		if (mma->write(mma, MMA8653FC_OFF_X, pdata->x_axis_offset))
> +			goto err_free_irq;
> +	}
> +	if (pdata->y_axis_offset) {
> +		if (mma->write(mma, MMA8653FC_OFF_Y, pdata->y_axis_offset))
> +			goto err_free_irq;
> +	}
> +	if (pdata->z_axis_offset) {
> +		if (mma->write(mma, MMA8653FC_OFF_Z, pdata->z_axis_offset))
> +			goto err_free_irq;
> +	}
> +
> +	/* if autosleep, wake on both landscape and motion changes */
> +	if (pdata->auto_wake_sleep) {
> +		byte = 0;
> +		byte |= WAKE_LNDPRT;
> +		byte |= WAKE_FF_MT;
> +		if (mma->write(mma, MMA8653FC_CTRL_REG3, byte))
> +			goto err_free_irq;
> +		if (mma->write(mma, MMA8653FC_CTRL_REG2, SLPE))
> +			goto err_free_irq;
> +		dev_dbg(&client->dev, "auto sleep enabled\n");
> +	}
> +
> +	/* data rates */
> +	byte = 0;
> +	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
> +	if (byte < 0)
> +		goto err_free_irq;
> +	byte &= ~ODR_MASK;
> +	byte |= ODR_DEFAULT;
> +	byte &= ~ASLP_RATE_MASK;
> +	byte |= ASLP_RATE_DEFAULT;
> +	if (mma->write(mma, MMA8653FC_CTRL_REG1, byte))
> +		goto err_free_irq;
> +
> +	/* freefall / motion config */
> +	byte = 0;
> +	if (pdata->motion_mode) {
> +		byte |= FF_MT_CFG_OAE;
> +		dev_dbg(&client->dev, "detect motion instead of freefall\n");
> +	}
> +	byte |= FF_MT_CFG_ELE;
> +	if (pdata->int_src_ff_mt_x)
> +		byte |= FF_MT_CFG_XEFE;
> +	if (pdata->int_src_ff_mt_y)
> +		byte |= FF_MT_CFG_YEFE;
> +	if (pdata->int_src_ff_mt_z)
> +		byte |= FF_MT_CFG_ZEFE;
> +	if (mma->write(mma, MMA8653FC_FF_MT_CFG, byte))
> +		goto err_free_irq;
> +
> +	if (pdata->freefall_motion_thr) {
> +		if (mma->write(mma, MMA8653FC_FF_MT_THS,
> +				 pdata->freefall_motion_thr))
> +			goto err_free_irq;
> +		/* calculate back to mg */
> +		dev_dbg(&client->dev, "threshold set to %dmg\n",
> +			 (63 * pdata->freefall_motion_thr) - 1);
> +	}
> +
> +	byte = mma->read(mma, MMA8653FC_WHO_AM_I);
> +	if (byte != MMA8653FC_DEVICE_ID) {
> +		dev_err(&client->dev, "wrong device for driver\n");
> +		goto err_free_irq;
> +	}
> +	dev_info(&client->dev,
> +		 "accelerometer driver loaded. device id %x\n", byte);
> +
> +	return 0;
> +
> + err_free_irq:
> +	if (mma->irq)
> +		free_irq(mma->irq, mma);
> + err_free_mem:
> +	kfree(mma);

If we use devm_* API's above steps are not required

> + err_out:
> +	return err;
> +}
> +
> +static int mma8653fc_remove(struct i2c_client *client)
> +{
> +	struct mma8653fc *mma = i2c_get_clientdata(client);
> +
> +	free_irq(mma->irq, mma);
> +	dev_dbg(&client->dev, "unregistered accelerometer\n");
> +	kfree(mma);

same as above..



-- 
Varka Bhadram


^ permalink raw reply

* Re: [PATCH v3] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Martin Kepplinger @ 2015-03-20 11:37 UTC (permalink / raw)
  To: Mark Rutland, Martin Kepplinger
  Cc: robh+dt@kernel.org, Pawel Moll, ijc+devicetree@hellion.org.uk,
	galak@codeaurora.org, dmitry.torokhov@gmail.com,
	alexander.stein@systec-electronic.com, hadess@hadess.net,
	akpm@linux-foundation.org, gregkh@linuxfoundation.org,
	linux-api@vger.kernel.org, devicetree@vger.kernel.org,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Christoph Muellner
In-Reply-To: <20150319160349.GD25967@leverpostej>

Am 2015-03-19 um 17:03 schrieb Mark Rutland:
>> diff --git a/Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt b/Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt
>> new file mode 100644
>> index 0000000..3921acb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/fsl,mma8653fc.txt
>> @@ -0,0 +1,96 @@
>> +Freescale MMA8653FC 3-axis Accelerometer
>> +
>> +Required properties:
>> +- compatible
>> +       "fsl,mma8653fc"
>> +- reg
>> +       I2C address
>> +
>> +Optional properties:
>> +
>> +- interrupt-parent
>> +       a phandle for the interrupt controller (see
>> +       Documentation/devicetree/bindings/interrupt-controller/interrupts.txt)
>> +- interrupts
>> +       interrupt line to which the chip is connected
>> +- int1
>> +       set to use interrupt line 1 instead of 2
> 
> If you have two interrupt output lines, you should have two entries in
> interrupts.
> 
> You can use interrupt-names to determine which line(s) are wired up.
> 

You can't use both interrupt lines. You have to decide on one and all
your selected interrupt sources will be on this one line.

> I don't believe that you need this property.
> 
>> +- int_active_high
>> +       set interrupt line active high
> 
> s/_/-/ in property names please. 
> 
> What happens if this isn't set? Is it active-low, or edge-triggered?
> 
> It feels like we should be able to query when we need to set this from
> the IRQ(s).

It's active low per default. I investigated and couldn't successfully
use active high. In v4 of the patch, this option is gone and documented.

> 
>> +- ir_freefall_motion_x
>> +       activate freefall/motion interrupts on x axis
>> +- ir_freefall_motion_y
>> +       activate freefall/motion interrupts on y axis
>> +- ir_freefall_motion_z
>> +       activate freefall/motion interrupts on z axis
>> +- irq_threshold
>> +       0 < value < 8000: threshold for motion interrupts in mg
>> +- ir_landscape_portrait
>> +       activate landscape/portrait interrupts
>> +- ir_data_ready:
>> +       activate data-ready interrupts
>> +       Interrupt events can be activated in any combination.
> 
> These all sounds like they would be better as runtime options. I don't
> see why these should necessarily be in the DT.

First, I don't really want to expand the sysfs ABI much. Second, there
are use cases that don't need these at runtime and it would seem to make
it even harder for systems to use this driver.

> 
>> +- range
>> +       2, 4, or 8: range in g, default: 2
> 
> Likewise.
> 
> It would be nice to have a better qualified name than "range".

I use "dynamic-range" in v4 of the patch.

> 
>> +- auto_wake_sleep
>> +       auto sleep mode (lower frequency)
>> +- motion_mode
>> +       use motion mode instead of freefall mode (trigger if >threshold).
>> +       per default an interrupt occurs if motion values fall below the
>> +       value set in "threshold" and therefore can detect free fall on the
>> +       vertical axis (depending on the position of the device).
>> +       Setting this values inverts the behaviour and an interrupt occurs
>> +       above the threshold value, so usually activate horizontal axis in
>> +       this case.
> 
> These both sound like they would be better as runtime options.
> 
>> +
>> +- x-offset
>> +       0 < value < 500: calibration offset in mg
>> +       this value has an offset of 250 itself:
>> +       0 is -250mg, 250 is 0 mg, 500 is 250mg
>> +- y-offset
>> +       see x-offset
>> +- z-offset
>> +       see x-offset
> 
> I'm unsure about these; it really depends on what the calibration is
> for.

this is better documented in v4 of the patch.

> 
> Mark.
> 

thanks for reviewing!

                                      martin

^ permalink raw reply

* Re: [PATCH v2] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Benjamin Tissoires @ 2015-03-20 12:27 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: Bastien Nocera, Martin Kepplinger, Alexander Stein,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, Dmitry Torokhov, Andrew Morton,
	Greg Kroah-Hartman, linux-api-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, linux-input,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Christoph Muellner
In-Reply-To: <550C03FC.1020303-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>

On Fri, Mar 20, 2015 at 7:26 AM, Martin Kepplinger
<martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org> wrote:
> Am 2015-03-19 um 11:22 schrieb Bastien Nocera:
>> On Wed, 2015-03-18 at 19:28 +0100, Martin Kepplinger wrote:
>>> Am 2015-03-18 um 19:05 schrieb Bastien Nocera:
>>>> On Wed, 2015-03-18 at 19:02 +0100, Martin Kepplinger wrote:
>>>>> Am 2015-03-18 um 17:59 schrieb Bastien Nocera:
>>>>>> On Wed, 2015-03-18 at 17:42 +0100, Martin Kepplinger wrote:
>>>>>>>
>>>>>> <snip>
>>>>>>> It could have gone to drivers/iio/accel if it would use an
>>>>>>> iio   interface, which would make more sense, you are right,
>>>>>>> but I
>>>>>>> simply  don't have the time to merge it in to iio.
>>>>>>>
>>>>>>> It doesn't use an input interface either but I don't see a
>>>>>>> good   place for an accelerometer that uses sysfs only.
>>>>>>>
>>>>>>> It works well, is a relatively recent chip and a clean
>>>>>>> dirver.  But  this is all I can provide.
>>>>>>
>>>>>> As a person who works on the user-space interaction of those
>>>>>> with   desktops [1]: Urgh.
>>>>>>
>>>>>> I already have 3 (probably 4) types of accelerometers to
>>>>>> contend  with,  I'm not fond of adding yet another type.
>>>>>>
>>>>>> Is there any way to get this hardware working outside the SoCs
>>>>>> it's  designed for (say, a device with I2C like a Raspberry
>>>>>> Pi),  so that a  kind soul could handle getting this using the
>>>>>> right  interfaces?
>>>>>>
>>>>>
>>>>> It works on basically any SoC and is in no way limited in this
>>>>> regard. Sure, userspace has to expicitely support it and I hear
>>>>> you.  Using the iio interface would make more sense. I can only
>>>>> say I'd  love to have the time to move this driver over. I'm
>>>>> very sorry.
>>>>
>>>> How can we get the hardware for somebody to use on their own
>>>> laptops/embedded boards to implement this driver?
>>>>
>>>
>>> It's connected over I2C. If the included documentation is not clear
>>> please tell me what exacly. Thanks!
>>
>>
>> I'll ask the question a different way: can you please give the address
>> of a shop where that hardware is available?
>>
>
> there is
> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=RDMMA865x&lang_cd=
> and http://linux-sunxi.org/Inet_K970 for example. I think I saw Android
> devices with it too, and I would guess it would be used more often if it
> were in linux.

I am going to say again (in a different way) what Bastien said. In its
current form, even in drivers/misc, this is a NACK for me (v1, v2, v3
& v4).

Putting a driver in Linux means we have to support it forever, and
definitively, nobody will use an accelerometer in Android if it is in
drivers/misc.
Android requires drivers to follow the IIO protocol. Period.
So having your own will not help android, it will just be a burden.

The sysfs you are proposing seems simple enough, but we can not afford
having a 3rd custom way of relying the accelerometer information in
the Linux tree (they were first handled in input, then IIO, then a
custom sysfs).

If you really want to have the driver in the tree, I won't be opposed
if you put in under staging. This way, you can break it whenever you
want and people won't rely on it. And then, we can use your driver as
a base to port it to IIO.

Sorry for being rude, but I am starting to get tired of people saying
that they don't have the time to follow what the reviewers said. You
obviously spent some time polishing this driver, why not making it
right from its first inclusion in the tree?

Cheers,
Benjamin

>
> Please refer to v4 of the patch for different questions. thanks!
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [Patch] firmware: dmi_scan: split dmisubsystem from dmi-sysfs
From: Jean Delvare @ 2015-03-20 12:35 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Ivan.khoronzhuk, linux-kernel, ard.biesheuvel, grant.likely,
	linux-api, linux-doc, mikew, dmidecode-devel, leif.lindholm,
	msalter
In-Reply-To: <1426852834.2727.25.camel@intel.com>

On Fri, 20 Mar 2015 12:00:34 +0000, Matt Fleming wrote:
> On Fri, 2015-03-20 at 09:16 +0100, Jean Delvare wrote:
> > 
> > OK, I understand. Now I see the two patches I was missing, I'll pick
> > them so that I can apply your patch cleanly on top of my tree.
> > 
> > I would have appreciated to be Cc'd on these two patches, so I knew
> > they existed, and maybe I could even have commented on them.
> 
> Jean, I would suggest you add your name to an entry in MAINTAINERS if
> you want to pickup these patches or be Cc'd on them.

Yeah, I had the same thought when writing the above. Will do.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply

* [PATCH v2] Add virtio-input driver.
From: Gerd Hoffmann @ 2015-03-20 12:39 UTC (permalink / raw)
  To: virtio-dev-sDuHXQ4OtrM4h7I2RyI4rWD2FQJk+8+b,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: mst-H+wXaHxf7aLQT0dZR+AlfA, David Herrmann, Dmitry Torokhov,
	Gerd Hoffmann, Rusty Russell, open list, open list:ABI/API

virtio-input is basically evdev-events-over-virtio, so this driver isn't
much more than reading configuration from config space and forwarding
incoming events to the linux input layer.

Signed-off-by: Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 MAINTAINERS                       |   6 +
 drivers/virtio/Kconfig            |  10 ++
 drivers/virtio/Makefile           |   1 +
 drivers/virtio/virtio_input.c     | 335 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/Kbuild         |   1 +
 include/uapi/linux/virtio_ids.h   |   1 +
 include/uapi/linux/virtio_input.h |  75 +++++++++
 7 files changed, 429 insertions(+)
 create mode 100644 drivers/virtio/virtio_input.c
 create mode 100644 include/uapi/linux/virtio_input.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 0e1abe8..585e6cd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10435,6 +10435,12 @@ S:	Maintained
 F:	drivers/vhost/
 F:	include/uapi/linux/vhost.h
 
+VIRTIO INPUT DRIVER
+M:	Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+S:	Maintained
+F:	drivers/virtio/virtio_input.c
+F:	include/uapi/linux/virtio_input.h
+
 VIA RHINE NETWORK DRIVER
 M:	Roger Luethi <rl-7uj+XXdSDtwfv37vnLkPlQ@public.gmane.org>
 S:	Maintained
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index b546da5..cab9f3f 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -48,6 +48,16 @@ config VIRTIO_BALLOON
 
 	 If unsure, say M.
 
+config VIRTIO_INPUT
+	tristate "Virtio input driver"
+	depends on VIRTIO
+	depends on INPUT
+	---help---
+	 This driver supports virtio input devices such as
+	 keyboards, mice and tablets.
+
+	 If unsure, say M.
+
  config VIRTIO_MMIO
 	tristate "Platform bus driver for memory mapped virtio devices"
 	depends on HAS_IOMEM
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index d85565b..41e30e3 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
 virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
 virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
+obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
new file mode 100644
index 0000000..dd3215e
--- /dev/null
+++ b/drivers/virtio/virtio_input.c
@@ -0,0 +1,335 @@
+#include <linux/module.h>
+#include <linux/virtio.h>
+#include <linux/input.h>
+
+#include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_input.h>
+
+struct virtio_input {
+	struct virtio_device       *vdev;
+	struct input_dev           *idev;
+	char                       name[64];
+	char                       serial[64];
+	char                       phys[64];
+	struct virtqueue           *evt, *sts;
+	struct virtio_input_event  evts[64];
+	struct mutex               lock;
+};
+
+static ssize_t serial_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct input_dev *idev = to_input_dev(dev);
+	struct virtio_input *vi = input_get_drvdata(idev);
+
+	return sprintf(buf, "%s\n", vi->serial);
+}
+static DEVICE_ATTR_RO(serial);
+
+static struct attribute *dev_attrs[] = {
+	&dev_attr_serial.attr,
+	NULL
+};
+
+static umode_t dev_attrs_are_visible(struct kobject *kobj,
+				     struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct input_dev *idev = to_input_dev(dev);
+	struct virtio_input *vi = input_get_drvdata(idev);
+
+	if (a == &dev_attr_serial.attr && !strlen(vi->serial))
+		return 0;
+
+	return a->mode;
+}
+
+static struct attribute_group dev_attr_grp = {
+	.attrs =	dev_attrs,
+	.is_visible =	dev_attrs_are_visible,
+};
+
+static const struct attribute_group *dev_attr_groups[] = {
+	&dev_attr_grp,
+	NULL
+};
+
+static void virtinput_queue_evtbuf(struct virtio_input *vi,
+				   struct virtio_input_event *evtbuf)
+{
+	struct scatterlist sg[1];
+
+	sg_init_one(sg, evtbuf, sizeof(*evtbuf));
+	virtqueue_add_inbuf(vi->evt, sg, 1, evtbuf, GFP_ATOMIC);
+}
+
+static void virtinput_recv_events(struct virtqueue *vq)
+{
+	struct virtio_input *vi = vq->vdev->priv;
+	struct virtio_input_event *event;
+	unsigned int len;
+
+	mutex_lock(&vi->lock);
+	while ((event = virtqueue_get_buf(vi->evt, &len)) != NULL) {
+		input_event(vi->idev,
+			    le16_to_cpu(event->type),
+			    le16_to_cpu(event->code),
+			    le32_to_cpu(event->value));
+		virtinput_queue_evtbuf(vi, event);
+	}
+	virtqueue_kick(vq);
+	mutex_unlock(&vi->lock);
+}
+
+static int virtinput_send_status(struct virtio_input *vi,
+				 u16 type, u16 code, s32 value)
+{
+	struct virtio_input_event *stsbuf;
+	struct scatterlist sg[1];
+	int rc;
+
+	stsbuf = kzalloc(sizeof(*stsbuf), GFP_ATOMIC);
+	if (!stsbuf)
+		return -ENOMEM;
+
+	stsbuf->type  = cpu_to_le16(type);
+	stsbuf->code  = cpu_to_le16(code);
+	stsbuf->value = cpu_to_le32(value);
+	sg_init_one(sg, stsbuf, sizeof(*stsbuf));
+
+	mutex_lock(&vi->lock);
+	rc = virtqueue_add_outbuf(vi->sts, sg, 1, stsbuf, GFP_ATOMIC);
+	virtqueue_kick(vi->sts);
+	mutex_unlock(&vi->lock);
+
+	if (rc != 0)
+		kfree(stsbuf);
+	return rc;
+}
+
+static void virtinput_recv_status(struct virtqueue *vq)
+{
+	struct virtio_input *vi = vq->vdev->priv;
+	struct virtio_input_event *stsbuf;
+	unsigned int len;
+
+	mutex_lock(&vi->lock);
+	while ((stsbuf = virtqueue_get_buf(vi->sts, &len)) != NULL)
+		kfree(stsbuf);
+	mutex_unlock(&vi->lock);
+}
+
+static int virtinput_status(struct input_dev *idev, unsigned int type,
+			    unsigned int code, int value)
+{
+	struct virtio_input *vi = input_get_drvdata(idev);
+
+	return virtinput_send_status(vi, type, code, value);
+}
+
+static size_t virtinput_cfg_select(struct virtio_input *vi,
+				   u8 select, u8 subsel)
+{
+	u8 size;
+
+	virtio_cwrite(vi->vdev, struct virtio_input_config, select, &select);
+	virtio_cwrite(vi->vdev, struct virtio_input_config, subsel, &subsel);
+	virtio_cread(vi->vdev, struct virtio_input_config, size, &size);
+	return size;
+}
+
+static void virtinput_cfg_bits(struct virtio_input *vi, int select, int subsel,
+			       unsigned long *bits, unsigned int bitcount)
+{
+	unsigned int bit;
+	size_t bytes;
+	u8 cfg = 0;
+
+	bytes = virtinput_cfg_select(vi, select, subsel);
+	if (!bytes)
+		return;
+	if (bitcount > bytes*8)
+		bitcount = bytes*8;
+	if (select == VIRTIO_INPUT_CFG_EV_BITS)
+		set_bit(subsel, vi->idev->evbit);
+	for (bit = 0; bit < bitcount; bit++) {
+		if ((bit % 8) == 0)
+			virtio_cread(vi->vdev, struct virtio_input_config,
+				     u.bitmap[bit / 8], &cfg);
+		if (cfg & (1 << (bit % 8)))
+			set_bit(bit, bits);
+	}
+}
+
+static void virtinput_cfg_abs(struct virtio_input *vi, int abs)
+{
+	u32 mi, ma, fu, fl;
+
+	virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
+	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
+	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
+	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
+	virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
+	input_set_abs_params(vi->idev, abs, mi, ma, fu, fl);
+}
+
+static int virtinput_init_vqs(struct virtio_input *vi)
+{
+	struct virtqueue *vqs[2];
+	vq_callback_t *cbs[] = { virtinput_recv_events,
+				 virtinput_recv_status };
+	static const char * names[] = { "events", "status" };
+	int i, err, size;
+
+	err = vi->vdev->config->find_vqs(vi->vdev, 2, vqs, cbs, names);
+	if (err)
+		return err;
+	vi->evt = vqs[0];
+	vi->sts = vqs[1];
+
+	size = virtqueue_get_vring_size(vi->evt);
+	if (size > ARRAY_SIZE(vi->evts))
+		size = ARRAY_SIZE(vi->evts);
+	for (i = 0; i < size; i++)
+		virtinput_queue_evtbuf(vi, &vi->evts[i]);
+	virtqueue_kick(vi->evt);
+
+	return 0;
+}
+
+static int virtinput_probe(struct virtio_device *vdev)
+{
+	struct virtio_input *vi;
+	size_t size;
+	int abs, err;
+
+	vi = kzalloc(sizeof(*vi), GFP_KERNEL);
+	if (!vi)
+		return -ENOMEM;
+
+	vdev->priv = vi;
+	vi->vdev = vdev;
+	mutex_init(&vi->lock);
+
+	err = virtinput_init_vqs(vi);
+	if (err)
+		goto err_init_vq;
+
+	vi->idev = input_allocate_device();
+	if (!vi->idev) {
+		err = -ENOMEM;
+		goto err_input_alloc;
+	}
+	input_set_drvdata(vi->idev, vi);
+
+	size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_NAME, 0);
+	virtio_cread_bytes(vi->vdev, offsetof(struct virtio_input_config, u),
+			   vi->name, min(size, sizeof(vi->name)));
+	size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_SERIAL, 0);
+	virtio_cread_bytes(vi->vdev, offsetof(struct virtio_input_config, u),
+			   vi->serial, min(size, sizeof(vi->serial)));
+	snprintf(vi->phys, sizeof(vi->phys),
+		 "virtio%d/input0", vdev->index);
+	vi->idev->name = vi->name;
+	vi->idev->phys = vi->phys;
+	vi->idev->uniq = vi->serial;
+
+	size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_DEVIDS, 0);
+	if (size >= 8) {
+		virtio_cread(vi->vdev, struct virtio_input_config,
+			     u.ids.bustype, &vi->idev->id.bustype);
+		virtio_cread(vi->vdev, struct virtio_input_config,
+			     u.ids.vendor, &vi->idev->id.vendor);
+		virtio_cread(vi->vdev, struct virtio_input_config,
+			     u.ids.product, &vi->idev->id.product);
+		virtio_cread(vi->vdev, struct virtio_input_config,
+			     u.ids.version, &vi->idev->id.version);
+	} else {
+		vi->idev->id.bustype = BUS_VIRTUAL;
+	}
+
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_PROP_BITS, 0,
+			   vi->idev->propbit, INPUT_PROP_CNT);
+	size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_REP);
+	if (size)
+		set_bit(EV_REP, vi->idev->evbit);
+
+	vi->idev->dev.parent = &vdev->dev;
+	vi->idev->dev.groups = dev_attr_groups;
+	vi->idev->event = virtinput_status;
+
+	/* device -> kernel */
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_KEY,
+			   vi->idev->keybit, KEY_CNT);
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_REL,
+			   vi->idev->relbit, REL_CNT);
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_ABS,
+			   vi->idev->absbit, ABS_CNT);
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_MSC,
+			   vi->idev->mscbit, MSC_CNT);
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_SW,
+			   vi->idev->swbit,  SW_CNT);
+
+	/* kernel -> device */
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_LED,
+			   vi->idev->ledbit, LED_CNT);
+	virtinput_cfg_bits(vi, VIRTIO_INPUT_CFG_EV_BITS, EV_SND,
+			   vi->idev->sndbit, SND_CNT);
+
+	if (test_bit(EV_ABS, vi->idev->evbit)) {
+		for (abs = 0; abs < ABS_CNT; abs++) {
+			if (!test_bit(abs, vi->idev->absbit))
+				continue;
+			virtinput_cfg_abs(vi, abs);
+		}
+	}
+	virtio_device_ready(vdev);
+
+	err = input_register_device(vi->idev);
+	if (err)
+		goto err_input_register;
+
+	return 0;
+
+err_input_register:
+	input_free_device(vi->idev);
+err_input_alloc:
+	vdev->config->del_vqs(vdev);
+err_init_vq:
+	kfree(vi);
+	return err;
+}
+
+static void virtinput_remove(struct virtio_device *vdev)
+{
+	struct virtio_input *vi = vdev->priv;
+
+	input_unregister_device(vi->idev);
+	input_free_device(vi->idev);
+	vdev->config->del_vqs(vdev);
+	kfree(vi);
+}
+
+static unsigned int features[] = {
+};
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_INPUT, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static struct virtio_driver virtio_input_driver = {
+	.driver.name         = KBUILD_MODNAME,
+	.driver.owner        = THIS_MODULE,
+	.feature_table       = features,
+	.feature_table_size  = ARRAY_SIZE(features),
+	.id_table            = id_table,
+	.probe               = virtinput_probe,
+	.remove              = virtinput_remove,
+};
+
+module_virtio_driver(virtio_input_driver);
+MODULE_DEVICE_TABLE(virtio, id_table);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Virtio input device driver");
+MODULE_AUTHOR("Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>");
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 68ceb97..04b829e 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -430,6 +430,7 @@ header-y += virtio_blk.h
 header-y += virtio_config.h
 header-y += virtio_console.h
 header-y += virtio_ids.h
+header-y += virtio_input.h
 header-y += virtio_net.h
 header-y += virtio_pci.h
 header-y += virtio_ring.h
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 284fc3a..5f60aa4 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -39,5 +39,6 @@
 #define VIRTIO_ID_9P		9 /* 9p virtio console */
 #define VIRTIO_ID_RPROC_SERIAL 11 /* virtio remoteproc serial link */
 #define VIRTIO_ID_CAIF	       12 /* Virtio caif */
+#define VIRTIO_ID_INPUT        18 /* virtio input */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/uapi/linux/virtio_input.h b/include/uapi/linux/virtio_input.h
new file mode 100644
index 0000000..9302422
--- /dev/null
+++ b/include/uapi/linux/virtio_input.h
@@ -0,0 +1,75 @@
+#ifndef _LINUX_VIRTIO_INPUT_H
+#define _LINUX_VIRTIO_INPUT_H
+/* This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE. */
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+
+enum virtio_input_config_select {
+	VIRTIO_INPUT_CFG_UNSET      = 0x00,
+	VIRTIO_INPUT_CFG_ID_NAME    = 0x01,
+	VIRTIO_INPUT_CFG_ID_SERIAL  = 0x02,
+	VIRTIO_INPUT_CFG_ID_DEVIDS  = 0x03,
+	VIRTIO_INPUT_CFG_PROP_BITS  = 0x10,
+	VIRTIO_INPUT_CFG_EV_BITS    = 0x11,
+	VIRTIO_INPUT_CFG_ABS_INFO   = 0x12,
+};
+
+struct virtio_input_absinfo {
+	__u32  min;
+	__u32  max;
+	__u32  fuzz;
+	__u32  flat;
+};
+
+struct virtio_input_devids {
+	__u16  bustype;
+	__u16  vendor;
+	__u16  product;
+	__u16  version;
+};
+
+struct virtio_input_config {
+	__u8    select;
+	__u8    subsel;
+	__u8    size;
+	__u8    reserved;
+	union {
+		char string[128];
+		__u8 bitmap[128];
+		struct virtio_input_absinfo abs;
+		struct virtio_input_devids ids;
+	} u;
+};
+
+struct virtio_input_event {
+	__le16 type;
+	__le16 code;
+	__le32 value;
+};
+
+#endif /* _LINUX_VIRTIO_INPUT_H */
-- 
1.8.3.1

^ permalink raw reply related

* Re: [v10 3/5] ext4: adds project quota support
From: Li Xi @ 2015-03-20 13:02 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Ext4 Developers List,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Theodore Ts'o, Jan Kara,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, Dmitry Monakhov
In-Reply-To: <D6B2436D-287C-4CC2-8646-8FF0A9F74E07-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>

On Fri, Mar 20, 2015 at 3:23 AM, Andreas Dilger <adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org> wrote:
> On Mar 18, 2015, at 1:04 PM, Li Xi <pkuelelixi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> This patch adds mount options for enabling/disabling project quota
>> accounting and enforcement. A new specific inode is also used for
>> project quota accounting.
>>
>> Signed-off-by: Li Xi <lixi-LfVdkaOWEx8@public.gmane.org>
>> Signed-off-by: Dmitry Monakhov <dmonakhov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
>> Reviewed-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
>> ---
>> fs/ext4/ext4.h  |    9 +++++-
>> fs/ext4/super.c |   76 ++++++++++++++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 74 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 7acb2da..ad650d4 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -208,6 +208,7 @@ struct ext4_io_submit {
>> #define EXT4_UNDEL_DIR_INO     6      /* Undelete directory inode */
>> #define EXT4_RESIZE_INO                7      /* Reserved group descriptors inode */
>> #define EXT4_JOURNAL_INO       8      /* Journal inode */
>> +#define EXT4_PRJ_QUOTA_INO   11      /* Project quota inode */
>
> To me it doesn't make sense to reserve this inode number if we aren't
> going to use it for project quota.  I think it is just confusing for
> developers that incorrectly think this is correct instead of checking
> s_prj_quota_inum.
Yeah, I agree that this definition is confusing and it is not so useful since
it can be replaced by s_prj_quota_inum. I will removing this definition.
In my current patches for E2fsprogs, project quota still uses inode number
11. And if project quota feature is enabled, the s_first_ino will be set to
s_prj_quota_inum + 1. I hope this is a acceptable implementation.
>
>> /* First non-reserved inode for old ext4 filesystems */
>> #define EXT4_GOOD_OLD_FIRST_INO       11
>> @@ -987,6 +988,7 @@ struct ext4_inode_info {
>> #define EXT4_MOUNT_DIOREAD_NOLOCK     0x400000 /* Enable support for dio read nolocking */
>> #define EXT4_MOUNT_JOURNAL_CHECKSUM   0x800000 /* Journal checksums */
>> #define EXT4_MOUNT_JOURNAL_ASYNC_COMMIT       0x1000000 /* Journal Async Commit */
>> +#define EXT4_MOUNT_PRJJQUOTA         0x2000000 /* Journaled Project quota */
>> #define EXT4_MOUNT_DELALLOC           0x8000000 /* Delalloc support */
>> #define EXT4_MOUNT_DATA_ERR_ABORT     0x10000000 /* Abort on file data write */
>> #define EXT4_MOUNT_BLOCK_VALIDITY     0x20000000 /* Block validity checking */
>> @@ -1169,7 +1171,8 @@ struct ext4_super_block {
>>       __le32  s_overhead_clusters;    /* overhead blocks/clusters in fs */
>>       __le32  s_backup_bgs[2];        /* groups with sparse_super2 SBs */
>>       __u8    s_encrypt_algos[4];     /* Encryption algorithms in use  */
>> -     __le32  s_reserved[105];        /* Padding to the end of the block */
>> +     __le32  s_prj_quota_inum;       /* inode for tracking project quota */
>> +     __le32  s_reserved[104];        /* Padding to the end of the block */
>>       __le32  s_checksum;             /* crc32c(superblock) */
>> };
>>
>> @@ -1184,7 +1187,7 @@ struct ext4_super_block {
>> #define EXT4_MF_FS_ABORTED    0x0002  /* Fatal error detected */
>>
>> /* Number of quota types we support */
>> -#define EXT4_MAXQUOTAS 2
>> +#define EXT4_MAXQUOTAS 3
>>
>> /*
>>  * fourth extended-fs super-block data in memory
>> @@ -1376,6 +1379,8 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
>>               ino == EXT4_BOOT_LOADER_INO ||
>>               ino == EXT4_JOURNAL_INO ||
>>               ino == EXT4_RESIZE_INO ||
>> +             (EXT4_FIRST_INO(sb) > EXT4_PRJ_QUOTA_INO &&
>> +              ino == EXT4_PRJ_QUOTA_INO) ||
>
> This check isn't correct either, if s_prj_quota_inum != EXT4_PRJ_QUOTA_INO.
>
> Cheers, Andreas
>
>>               (ino >= EXT4_FIRST_INO(sb) &&
>>                ino <= le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count));
>> }
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 04c6cc3..4077932 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1036,8 +1036,8 @@ static int bdev_try_to_free_page(struct super_block *sb, struct page *page,
>> }
>>
>> #ifdef CONFIG_QUOTA
>> -#define QTYPE2NAME(t) ((t) == USRQUOTA ? "user" : "group")
>> -#define QTYPE2MOPT(on, t) ((t) == USRQUOTA?((on)##USRJQUOTA):((on)##GRPJQUOTA))
>> +static char *quotatypes[] = INITQFNAMES;
>> +#define QTYPE2NAME(t) (quotatypes[t])
>>
>> static int ext4_write_dquot(struct dquot *dquot);
>> static int ext4_acquire_dquot(struct dquot *dquot);
>> @@ -1135,7 +1135,8 @@ enum {
>>       Opt_journal_path, Opt_journal_checksum, Opt_journal_async_commit,
>>       Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
>>       Opt_data_err_abort, Opt_data_err_ignore,
>> -     Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
>> +     Opt_usrjquota, Opt_grpjquota, Opt_prjjquota,
>> +     Opt_offusrjquota, Opt_offgrpjquota, Opt_offprjjquota,
>>       Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
>>       Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
>>       Opt_usrquota, Opt_grpquota, Opt_i_version,
>> @@ -1190,6 +1191,8 @@ static const match_table_t tokens = {
>>       {Opt_usrjquota, "usrjquota=%s"},
>>       {Opt_offgrpjquota, "grpjquota="},
>>       {Opt_grpjquota, "grpjquota=%s"},
>> +     {Opt_prjjquota, "prjjquota"},
>> +     {Opt_offprjjquota, "offprjjquota"},
>>       {Opt_jqfmt_vfsold, "jqfmt=vfsold"},
>>       {Opt_jqfmt_vfsv0, "jqfmt=vfsv0"},
>>       {Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
>> @@ -1412,11 +1415,16 @@ static const struct mount_opts {
>>       {Opt_grpquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_GRPQUOTA,
>>                                                       MOPT_SET | MOPT_Q},
>>       {Opt_noquota, (EXT4_MOUNT_QUOTA | EXT4_MOUNT_USRQUOTA |
>> -                    EXT4_MOUNT_GRPQUOTA), MOPT_CLEAR | MOPT_Q},
>> +                    EXT4_MOUNT_GRPQUOTA | EXT4_MOUNT_PRJJQUOTA),
>> +                                                     MOPT_CLEAR | MOPT_Q},
>>       {Opt_usrjquota, 0, MOPT_Q},
>>       {Opt_grpjquota, 0, MOPT_Q},
>> +     {Opt_prjjquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_PRJJQUOTA,
>> +                                                     MOPT_SET | MOPT_Q},
>>       {Opt_offusrjquota, 0, MOPT_Q},
>>       {Opt_offgrpjquota, 0, MOPT_Q},
>> +     {Opt_offprjjquota, 0, EXT4_MOUNT_PRJJQUOTA,
>> +                                                     MOPT_CLEAR | MOPT_Q},
>>       {Opt_jqfmt_vfsold, QFMT_VFS_OLD, MOPT_QFMT},
>>       {Opt_jqfmt_vfsv0, QFMT_VFS_V0, MOPT_QFMT},
>>       {Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
>> @@ -1673,7 +1681,9 @@ static int parse_options(char *options, struct super_block *sb,
>>                        "feature is enabled");
>>               return 0;
>>       }
>> -     if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
>> +     if (sbi->s_qf_names[USRQUOTA] ||
>> +         sbi->s_qf_names[GRPQUOTA] ||
>> +         test_opt(sb, PRJJQUOTA)) {
>>               if (test_opt(sb, USRQUOTA) && sbi->s_qf_names[USRQUOTA])
>>                       clear_opt(sb, USRQUOTA);
>>
>> @@ -3944,7 +3954,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>>               sb->s_qcop = &ext4_qctl_sysfile_operations;
>>       else
>>               sb->s_qcop = &ext4_qctl_operations;
>> -     sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
>> +     sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
>> #endif
>>       memcpy(sb->s_uuid, es->s_uuid, sizeof(es->s_uuid));
>>
>> @@ -5040,6 +5050,46 @@ restore_opts:
>>       return err;
>> }
>>
>> +static int ext4_statfs_project(struct super_block *sb,
>> +                            kprojid_t projid, struct kstatfs *buf)
>> +{
>> +     struct kqid qid;
>> +     struct dquot *dquot;
>> +     u64 limit;
>> +     u64 curblock;
>> +
>> +     qid = make_kqid_projid(projid);
>> +     dquot = dqget(sb, qid);
>> +     if (!dquot)
>> +             return -ESRCH;
>> +     spin_lock(&dq_data_lock);
>> +
>> +     limit = dquot->dq_dqb.dqb_bsoftlimit ?
>> +             dquot->dq_dqb.dqb_bsoftlimit :
>> +             dquot->dq_dqb.dqb_bhardlimit;
>> +     if (limit && buf->f_blocks * buf->f_bsize > limit) {
>> +             curblock = dquot->dq_dqb.dqb_curspace / buf->f_bsize;
>> +             buf->f_blocks = limit / buf->f_bsize;
>> +             buf->f_bfree = buf->f_bavail =
>> +                     (buf->f_blocks > curblock) ?
>> +                      (buf->f_blocks - curblock) : 0;
>> +     }
>> +
>> +     limit = dquot->dq_dqb.dqb_isoftlimit ?
>> +             dquot->dq_dqb.dqb_isoftlimit :
>> +             dquot->dq_dqb.dqb_ihardlimit;
>> +     if (limit && buf->f_files > limit) {
>> +             buf->f_files = limit;
>> +             buf->f_ffree =
>> +                     (buf->f_files > dquot->dq_dqb.dqb_curinodes) ?
>> +                      (buf->f_files - dquot->dq_dqb.dqb_curinodes) : 0;
>> +     }
>> +
>> +     spin_unlock(&dq_data_lock);
>> +     dqput(dquot);
>> +     return 0;
>> +}
>> +
>> static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
>> {
>>       struct super_block *sb = dentry->d_sb;
>> @@ -5048,6 +5098,7 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
>>       ext4_fsblk_t overhead = 0, resv_blocks;
>>       u64 fsid;
>>       s64 bfree;
>> +     struct inode *inode = dentry->d_inode;
>>       resv_blocks = EXT4_C2B(sbi, atomic64_read(&sbi->s_resv_clusters));
>>
>>       if (!test_opt(sb, MINIX_DF))
>> @@ -5072,6 +5123,9 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
>>       buf->f_fsid.val[0] = fsid & 0xFFFFFFFFUL;
>>       buf->f_fsid.val[1] = (fsid >> 32) & 0xFFFFFFFFUL;
>>
>> +     if (ext4_test_inode_flag(inode, EXT4_INODE_PROJINHERIT) &&
>> +         sb_has_quota_limits_enabled(sb, PRJQUOTA))
>> +             ext4_statfs_project(sb, EXT4_I(inode)->i_projid, buf);
>>       return 0;
>> }
>>
>> @@ -5152,7 +5206,9 @@ static int ext4_mark_dquot_dirty(struct dquot *dquot)
>>
>>       /* Are we journaling quotas? */
>>       if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_QUOTA) ||
>> -         sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
>> +         sbi->s_qf_names[USRQUOTA] ||
>> +         sbi->s_qf_names[GRPQUOTA] ||
>> +         test_opt(sb, PRJJQUOTA)) {
>>               dquot_mark_dquot_dirty(dquot);
>>               return ext4_write_dquot(dquot);
>>       } else {
>> @@ -5236,7 +5292,8 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
>>       struct inode *qf_inode;
>>       unsigned long qf_inums[EXT4_MAXQUOTAS] = {
>>               le32_to_cpu(EXT4_SB(sb)->s_es->s_usr_quota_inum),
>> -             le32_to_cpu(EXT4_SB(sb)->s_es->s_grp_quota_inum)
>> +             le32_to_cpu(EXT4_SB(sb)->s_es->s_grp_quota_inum),
>> +             le32_to_cpu(EXT4_SB(sb)->s_es->s_prj_quota_inum)
>>       };
>>
>>       BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_QUOTA));
>> @@ -5264,7 +5321,8 @@ static int ext4_enable_quotas(struct super_block *sb)
>>       int type, err = 0;
>>       unsigned long qf_inums[EXT4_MAXQUOTAS] = {
>>               le32_to_cpu(EXT4_SB(sb)->s_es->s_usr_quota_inum),
>> -             le32_to_cpu(EXT4_SB(sb)->s_es->s_grp_quota_inum)
>> +             le32_to_cpu(EXT4_SB(sb)->s_es->s_grp_quota_inum),
>> +             le32_to_cpu(EXT4_SB(sb)->s_es->s_prj_quota_inum)
>>       };
>>
>>       sb_dqopt(sb)->flags |= DQUOT_QUOTA_SYS_FILE;
>> --
>> 1.7.1
>>
>
>
> Cheers, Andreas
>
>
>
>
>

^ permalink raw reply

* Re: [v10 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Li Xi @ 2015-03-20 13:24 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel@vger.kernel.org, Ext4 Developers List,
	linux-api@vger.kernel.org, Theodore Ts'o, Andreas Dilger,
	viro@zeniv.linux.org.uk, hch@infradead.org, Dmitry Monakhov
In-Reply-To: <20150319095634.GF28368@quack.suse.cz>

On Thu, Mar 19, 2015 at 5:56 PM, Jan Kara <jack@suse.cz> wrote:
> On Thu 19-03-15 04:04:56, Li Xi wrote:
>> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
>> support for ext4. The interface is kept consistent with
>> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
>   Two more comments below.
>
>>
>> Signed-off-by: Li Xi <lixi@ddn.com>
> ...
>> +static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>> +{
>> +     struct inode *inode = file_inode(filp);
>> +     struct super_block *sb = inode->i_sb;
>> +     struct ext4_inode_info *ei = EXT4_I(inode);
>> +     int err;
>> +     handle_t *handle;
>> +     kprojid_t kprojid;
>> +     struct ext4_iloc iloc;
>> +     struct ext4_inode *raw_inode;
>> +
>> +     struct dquot *transfer_to[EXT4_MAXQUOTAS] = { };
>> +
>> +     /* Make sure caller can change project. */
>> +     if (!capable(CAP_SYS_ADMIN))
>> +             return -EACCES;
>   Why do you test capabilities here when you already test permission in
> EXT4_IOC_FSSETXATTR? Furthermore this test is too restrictive. Just delete
> it.
It seems we need this restrictive permission. Otherwise the owner of the file
can change the project ID to any other project. That means, the owner can
walk around the quota limit whenever he/she wants. But I agree checking
permission twice is not good.
> ...
>> -flags_out:
>> +             err = ext4_ioctl_setflags(inode, flags);
>>               mutex_unlock(&inode->i_mutex);
>> -             mnt_drop_write_file(filp);
>> +             mnt_drop_write(filp->f_path.mnt);
>   Why did you change this? mnt_drop_write_file() should stay here.
Sorry, my mistake.

Regards,
Li Xi

^ permalink raw reply

* Re: [PATCH] audit.h: remove the macro AUDIT_ARCH_ARMEB definition
From: Paul Moore @ 2015-03-20 13:29 UTC (permalink / raw)
  To: roy.qing.li-Re5JQEeQqe8AvxtiuMwx3w
  Cc: Eric Paris, linux-audit-H+wXaHxf7aLQT0dZR+AlfA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1426827329-27976-1-git-send-email-roy.qing.li-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri, Mar 20, 2015 at 12:55 AM,  <roy.qing.li-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Li RongQing <roy.qing.li-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> After 2f9783669 [ARM: 7412/1: audit: use only AUDIT_ARCH_ARM regardless
> of endianness], no kernel user uses this macro;
>
> Keeping this macro, only makes the compiling old version audit [before
> changeset 931 Improve ARM and AARCH64 support] success, but the audit
> program can not work with the kernel after 2f9783669 still,
> since no syscall entry is enabled for AUDIT_ARCH_ARMEB in kernel.
>
> so remove it to force to use the latest audit program
>
> Signed-off-by: Li RongQing <roy.qing.li-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> other workaround is to define AUDIT_ARCH_ARMEB as AUDIT_ARCH_ARM,
> but it seems very strange
>
>  include/uapi/linux/audit.h | 1 -
>  1 file changed, 1 deletion(-)

Since this #define lives in the user visible headers I don't want to
remove it and risk causing a userspace breakage.  Leaving the #define
in the header, even if it is unused by modern userspace, is harmless.

> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index d3475e1..125aa49 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -351,7 +351,6 @@ enum {
>  #define AUDIT_ARCH_AARCH64     (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>  #define AUDIT_ARCH_ALPHA       (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>  #define AUDIT_ARCH_ARM         (EM_ARM|__AUDIT_ARCH_LE)
> -#define AUDIT_ARCH_ARMEB       (EM_ARM)
>  #define AUDIT_ARCH_CRIS                (EM_CRIS|__AUDIT_ARCH_LE)
>  #define AUDIT_ARCH_FRV         (EM_FRV)
>  #define AUDIT_ARCH_I386                (EM_386|__AUDIT_ARCH_LE)
> --
> 2.1.0
>



-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH v4] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Martin Kepplinger @ 2015-03-20 13:43 UTC (permalink / raw)
  To: Varka Bhadram, mark.rutland, robh+dt, Pawel.Moll, ijc+devicetree,
	galak, dmitry.torokhov, alexander.stein
  Cc: hadess, akpm, gregkh, linux-api, devicetree, linux-input,
	linux-kernel, Martin Kepplinger, Christoph Muellner
In-Reply-To: <550C0502.7090507@gmail.com>

Am 2015-03-20 um 12:31 schrieb Varka Bhadram:
> On 03/20/2015 04:50 PM, Martin Kepplinger wrote:
>> From: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>
( ...)
>> +    return 0;
>> +
>> + err_free_irq:
>> +    if (mma->irq)
>> +        free_irq(mma->irq, mma);
>> + err_free_mem:
>> +    kfree(mma);
> 
> If we use devm_* API's above steps are not required
> 
>> + err_out:
>> +    return err;
>> +}
>> +
>> +static int mma8653fc_remove(struct i2c_client *client)
>> +{
>> +    struct mma8653fc *mma = i2c_get_clientdata(client);
>> +
>> +    free_irq(mma->irq, mma);
>> +    dev_dbg(&client->dev, "unregistered accelerometer\n");
>> +    kfree(mma);
> 
> same as above..
> 

thanks!

^ permalink raw reply

* [PATCH V7] Allow compaction of unevictable pages
From: Eric B Munson @ 2015-03-20 13:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Eric B Munson, Vlastimil Babka, Thomas Gleixner,
	Christoph Lameter, Peter Zijlstra, Mel Gorman, David Rientjes,
	Rik van Riel, Michal Hocko, linux-doc, linux-rt-users, linux-mm,
	linux-api, linux-kernel

Currently, pages which are marked as unevictable are protected from
compaction, but not from other types of migration.  The POSIX real time
extension explicitly states that mlock() will prevent a major page
fault, but the spirit of this is that mlock() should give a process the
ability to control sources of latency, including minor page faults.
However, the mlock manpage only explicitly says that a locked page will
not be written to swap and this can cause some confusion.  The
compaction code today does not give a developer who wants to avoid swap
but wants to have large contiguous areas available any method to achieve
this state.  This patch introduces a sysctl for controlling compaction
behavior with respect to the unevictable lru.  Users that demand no page
faults after a page is present can set compact_unevictable_allowed to 0
and users who need the large contiguous areas can enable compaction on
locked memory by leaving the default value of 1.

To illustrate this problem I wrote a quick test program that mmaps a
large number of 1MB files filled with random data.  These maps are
created locked and read only.  Then every other mmap is unmapped and I
attempt to allocate huge pages to the static huge page pool.  When the
compact_unevictable_allowed sysctl is 0, I cannot allocate hugepages
after fragmenting memory.  When the value is set to 1, allocations
succeed.

Signed-off-by: Eric B Munson <emunson@akamai.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Christoph Lameter <cl@linux.com>
Acked-by: David Rientjes <rientjes@google.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Christoph Lameter <cl@linux.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: David Rientjes <rientjes@google.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: linux-doc@vger.kernel.org
Cc: linux-rt-users@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-api@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes from V6:
* Fixed changelog spelling

 Documentation/sysctl/vm.txt |   11 +++++++++++
 include/linux/compaction.h  |    1 +
 kernel/sysctl.c             |    9 +++++++++
 mm/compaction.c             |    7 +++++++
 4 files changed, 28 insertions(+)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index 902b457..9832ec5 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -21,6 +21,7 @@ Currently, these files are in /proc/sys/vm:
 - admin_reserve_kbytes
 - block_dump
 - compact_memory
+- compact_unevictable_allowed
 - dirty_background_bytes
 - dirty_background_ratio
 - dirty_bytes
@@ -106,6 +107,16 @@ huge pages although processes will also directly compact memory as required.
 
 ==============================================================
 
+compact_unevictable_allowed
+
+Available only when CONFIG_COMPACTION is set. When set to 1, compaction is
+allowed to examine the unevictable lru (mlocked pages) for pages to compact.
+This should be used on systems where stalls for minor page faults are an
+acceptable trade for large contiguous free memory.  Set to 0 to prevent
+compaction from moving pages that are unevictable.  Default value is 1.
+
+==============================================================
+
 dirty_background_bytes
 
 Contains the amount of dirty memory at which the background kernel
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index a014559..aa8f61c 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -34,6 +34,7 @@ extern int sysctl_compaction_handler(struct ctl_table *table, int write,
 extern int sysctl_extfrag_threshold;
 extern int sysctl_extfrag_handler(struct ctl_table *table, int write,
 			void __user *buffer, size_t *length, loff_t *ppos);
+extern int sysctl_compact_unevictable_allowed;
 
 extern int fragmentation_index(struct zone *zone, unsigned int order);
 extern unsigned long try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 88ea2d6..2f6c880 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1313,6 +1313,15 @@ static struct ctl_table vm_table[] = {
 		.extra1		= &min_extfrag_threshold,
 		.extra2		= &max_extfrag_threshold,
 	},
+	{
+		.procname	= "compact_unevictable_allowed",
+		.data		= &sysctl_compact_unevictable_allowed,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+		.extra1		= &zero,
+		.extra2		= &one,
+	},
 
 #endif /* CONFIG_COMPACTION */
 	{
diff --git a/mm/compaction.c b/mm/compaction.c
index 8c0d945..ad88a8c 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1047,6 +1047,12 @@ typedef enum {
 } isolate_migrate_t;
 
 /*
+ * Allow userspace to control policy on scanning the unevictable LRU for
+ * compactable pages.
+ */
+int sysctl_compact_unevictable_allowed __read_mostly = 1;
+
+/*
  * Isolate all pages that can be migrated from the first suitable block,
  * starting at the block pointed to by the migrate scanner pfn within
  * compact_control.
@@ -1057,6 +1063,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,
 	unsigned long low_pfn, end_pfn;
 	struct page *page;
 	const isolate_mode_t isolate_mode =
+		(sysctl_compact_unevictable_allowed ? ISOLATE_UNEVICTABLE : 0) |
 		(cc->mode == MIGRATE_ASYNC ? ISOLATE_ASYNC_MIGRATE : 0);
 
 	/*
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH v2] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Martin Kepplinger @ 2015-03-20 13:56 UTC (permalink / raw)
  To: Benjamin Tissoires, Martin Kepplinger
  Cc: Bastien Nocera, Alexander Stein, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, Dmitry Torokhov,
	Andrew Morton, Greg Kroah-Hartman, linux-api, devicetree,
	linux-input, linux-kernel@vger.kernel.org, Christoph Muellner
In-Reply-To: <CAN+gG=GGFptRo=FXLp7S_Y1+mwerzYNN+Sf+6ZCwSpmTU-w9ag@mail.gmail.com>

Am 2015-03-20 um 13:27 schrieb Benjamin Tissoires:
> On Fri, Mar 20, 2015 at 7:26 AM, Martin Kepplinger
> <martin.kepplinger@theobroma-systems.com> wrote:
>> Am 2015-03-19 um 11:22 schrieb Bastien Nocera:
>>> On Wed, 2015-03-18 at 19:28 +0100, Martin Kepplinger wrote:
>>>> Am 2015-03-18 um 19:05 schrieb Bastien Nocera:
>>>>> On Wed, 2015-03-18 at 19:02 +0100, Martin Kepplinger wrote:
>>>>>> Am 2015-03-18 um 17:59 schrieb Bastien Nocera:
>>>>>>> On Wed, 2015-03-18 at 17:42 +0100, Martin Kepplinger wrote:
>>>>>>>>
>>>>>>> <snip>
>>>>>>>> It could have gone to drivers/iio/accel if it would use an
>>>>>>>> iio   interface, which would make more sense, you are right,
>>>>>>>> but I
>>>>>>>> simply  don't have the time to merge it in to iio.
>>>>>>>>
>>>>>>>> It doesn't use an input interface either but I don't see a
>>>>>>>> good   place for an accelerometer that uses sysfs only.
>>>>>>>>
>>>>>>>> It works well, is a relatively recent chip and a clean
>>>>>>>> dirver.  But  this is all I can provide.
>>>>>>>
>>>>>>> As a person who works on the user-space interaction of those
>>>>>>> with   desktops [1]: Urgh.
>>>>>>>
>>>>>>> I already have 3 (probably 4) types of accelerometers to
>>>>>>> contend  with,  I'm not fond of adding yet another type.
>>>>>>>
>>>>>>> Is there any way to get this hardware working outside the SoCs
>>>>>>> it's  designed for (say, a device with I2C like a Raspberry
>>>>>>> Pi),  so that a  kind soul could handle getting this using the
>>>>>>> right  interfaces?
>>>>>>>
>>>>>>
>>>>>> It works on basically any SoC and is in no way limited in this
>>>>>> regard. Sure, userspace has to expicitely support it and I hear
>>>>>> you.  Using the iio interface would make more sense. I can only
>>>>>> say I'd  love to have the time to move this driver over. I'm
>>>>>> very sorry.
>>>>>
>>>>> How can we get the hardware for somebody to use on their own
>>>>> laptops/embedded boards to implement this driver?
>>>>>
>>>>
>>>> It's connected over I2C. If the included documentation is not clear
>>>> please tell me what exacly. Thanks!
>>>
>>>
>>> I'll ask the question a different way: can you please give the address
>>> of a shop where that hardware is available?
>>>
>>
>> there is
>> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=RDMMA865x&lang_cd=
>> and http://linux-sunxi.org/Inet_K970 for example. I think I saw Android
>> devices with it too, and I would guess it would be used more often if it
>> were in linux.
> 
> I am going to say again (in a different way) what Bastien said. In its
> current form, even in drivers/misc, this is a NACK for me (v1, v2, v3
> & v4).
> 
> Putting a driver in Linux means we have to support it forever, and
> definitively, nobody will use an accelerometer in Android if it is in
> drivers/misc.
> Android requires drivers to follow the IIO protocol. Period.
> So having your own will not help android, it will just be a burden.
> 
> The sysfs you are proposing seems simple enough, but we can not afford
> having a 3rd custom way of relying the accelerometer information in
> the Linux tree (they were first handled in input, then IIO, then a
> custom sysfs).
> 
> If you really want to have the driver in the tree, I won't be opposed
> if you put in under staging. This way, you can break it whenever you
> want and people won't rely on it. And then, we can use your driver as
> a base to port it to IIO.

That seems reasonable. I have prepared a (little more cleaned up) v5 of
the patch and moved it to staging, with a TODO file containing also the
current documentation. I hope to be able to do the iio integration
sometime "soon", but this could speed things up.

> 
> Sorry for being rude, but I am starting to get tired of people saying
> that they don't have the time to follow what the reviewers said. You
> obviously spent some time polishing this driver, why not making it
> right from its first inclusion in the tree?

That's totally fine. Of course iio is the way to go. I had the driver
before I really knew iio and this was lazyness (when I found
Documentation/ABI/testing), plus the desire to publish it before the
chip itself is deprecated.

> 
> Cheers,
> Benjamin
> 
>>
>> Please refer to v4 of the patch for different questions. thanks!
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v5] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Martin Kepplinger @ 2015-03-20 14:06 UTC (permalink / raw)
  To: mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	Pawel.Moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	alexander.stein-93q1YBGzJSMe9JSWTWOYM3xStJ4P+DSV,
	benjamin.tissoires-Re5JQEeQqe8AvxtiuMwx3w
  Cc: hadess-0MeiytkfxGOsTnJN9+BGXg,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Martin Kepplinger,
	Christoph Muellner

From: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>

The MMA8653FC is a low-power, three-axis, capacitive micromachined
accelerometer with 10 bits of resolution with flexible user-programmable
options.

Embedded interrupt functions enable overall power savings, by relieving the
host processor from continuously polling data, for example using the poll()
system call.

The device can be configured to generate wake-up interrupt signals from any
combination of the configurable embedded functions, enabling the MMA8653FC
to monitor events while remaining in a low-power mode during periods of
inactivity.

This driver provides devicetree properties to program the device's behaviour
and a simple, tested and documented sysfs interface. The data sheet and more
information is available on Freescale's website.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

patch revision history
......................
v5 clean up (suggested by Varka Bhadram) and move the driver to staging
v4 changes DT propery names, adds a missing interrupt source and removes
   the DT option to set interrupt line active high due to unsuccesful testing
v3 moves the driver from drivers/input/misc to drivers/misc
v2 corrects licensing and commit messages and adds appropriate recipients


 drivers/staging/Kconfig               |   2 +
 drivers/staging/mma8653fc/Kconfig     |  10 +
 drivers/staging/mma8653fc/Makefile    |   1 +
 drivers/staging/mma8653fc/TODO        | 146 ++++++
 drivers/staging/mma8653fc/mma8653fc.c | 864 ++++++++++++++++++++++++++++++++++
 5 files changed, 1023 insertions(+)
 create mode 100644 drivers/staging/mma8653fc/Kconfig
 create mode 100644 drivers/staging/mma8653fc/Makefile
 create mode 100644 drivers/staging/mma8653fc/TODO
 create mode 100644 drivers/staging/mma8653fc/mma8653fc.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 45baa83..661e5f5 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -108,4 +108,6 @@ source "drivers/staging/fbtft/Kconfig"
 
 source "drivers/staging/i2o/Kconfig"
 
+source "drivers/staging/mma8653fc/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/mma8653fc/Kconfig b/drivers/staging/mma8653fc/Kconfig
new file mode 100644
index 0000000..988451b
--- /dev/null
+++ b/drivers/staging/mma8653fc/Kconfig
@@ -0,0 +1,10 @@
+config MMA8653FC
+        tristate "MMA8653FC - Freescale's 3-Axis, 10-bit Digital Accelerometer"
+        depends on I2C
+        default n
+        help
+          Say Y here if you want to support Freescale's MMA8653FC Accelerometer
+          through I2C interface.
+
+          To compile this driver as a module, choose M here: the
+          module will be called mma8653fc.
diff --git a/drivers/staging/mma8653fc/Makefile b/drivers/staging/mma8653fc/Makefile
new file mode 100644
index 0000000..9a245a3
--- /dev/null
+++ b/drivers/staging/mma8653fc/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MMA8653FC)         += mma8653fc.o
diff --git a/drivers/staging/mma8653fc/TODO b/drivers/staging/mma8653fc/TODO
new file mode 100644
index 0000000..0a31225
--- /dev/null
+++ b/drivers/staging/mma8653fc/TODO
@@ -0,0 +1,146 @@
+- move to IIO device API. The current DT/sysfs interface is documented below
+
+Documentation/ABI/testing/...
+-----------------------------
+What:		/sys/bus/i2c/drivers/mma8653fc/*/standby
+Date:		March 2015
+Contact:	Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
+Description:
+		Write 0 to this in order to turn on the device, and 1 to turn
+		it off. Read to see if it is turned on or off.
+
+
+What:		/sys/bus/i2c/drivers/mma8653fc/*/currentmode
+Date:		March 2015
+Contact:	Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
+Description:
+		Reading this provides the current state of the device, read
+		directly from a register. This can be "standby", "wake" or
+		"sleep".
+
+
+What:		/sys/bus/i2c/drivers/mma8653fc/*/position
+Date:		March 2015
+Contact:	Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
+Description:
+		Read only. Without interrupts enabled gets current position
+		values by reading. Poll "position" with interrupt conditions
+		set, to get notified; see Documentation/.../fsl,mma8653fc.txt
+
+		position file format:
+		"x y z [landscape/portrait status] [front/back status]"
+
+		x y z values:
+			in mg
+		landscape/portrait status char:
+			r	landscape right
+			d	portrait down
+			u	portrait up
+			l	landscape left
+		front/back status char:
+			f	front facing
+			b	back facing
+
+
+Documentation/devicetree/bindings/...
+-------------------------------------
+Required properties:
+- compatible
+	"fsl,mma8653fc"
+- reg
+	I2C address
+
+Optional properties:
+
+- interrupt-parent
+	a phandle for the interrupt controller (see
+	Documentation/devicetree/bindings/interrupt-controller/interrupts.txt)
+- interrupts
+	interrupt line to which the chip is connected (active low)
+- int1
+	set to use interrupt line 1, default is line 2
+	the interrupt sources can be routed to one of the two lines
+- ir-freefall-motion-x
+	activate freefall/motion interrupts on x axis
+- ir-freefall-motion-y
+	activate freefall/motion interrupts on y axis
+- ir-freefall-motion-z
+	activate freefall/motion interrupts on z axis
+- irq-threshold
+	0 < value < 8000: threshold for motion interrupts in mg
+- ir-landscape-portrait
+	activate landscape/portrait interrupts
+- ir-auto-wake
+	activate wake/sleep change interrupts
+- ir-data-ready:
+	activate data-ready interrupts
+	Interrupt events can be activated in any combination.
+- dynamic-range
+	2, 4, or 8: dynamic measurement range in g, default: 2
+	In ±2 g mode, sensitivity = 256 counts/g.
+	In ±4 g mode, sensitivity = 128 counts/g.
+	In ±8 g mode, sensitivity = 64 counts/g.
+- auto-wake-sleep
+	auto sleep mode (lower frequency)
+- motion-mode
+	use motion mode instead of freefall mode (trigger if >threshold).
+	per default an interrupt occurs if motion values fall below the
+	value set in "threshold" and therefore can detect free fall on the
+	vertical axis (depending on the position of the device).
+	Setting this values inverts the behaviour and an interrupt occurs
+	above the threshold value, so usually activate horizontal axis in
+	this case.
+
+- x-offset
+	0 < value < 500: calibration offset in mg
+	The offset correction values are used to realign the Zero-g position
+	of the X, Y, and Z-axis after the device is mounted on a board.
+	this value has an offset of 250 itself:
+	0 is -250mg, 250 is 0 mg, 500 is 250mg
+- y-offset
+	see x-offset
+- z-offset
+	see x-offset
+
+Example 1:
+for a device laying on flat ground to recognize acceleration over 100mg.
+x-axis is calibrated to +10mg. Adapt interrupt line to your device.
+
+mma8653fc@1d {
+		compatible = "fsl,mma8653fc";
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+		reg = <0x1d>;
+
+		dynamic-range = <2>;
+		motion-mode;
+		ir-freefall-motion-x;
+		ir-freefall-motion-y;
+		irq-threshold = <100>;
+		x-offset = <160>;
+};
+
+Example 2:
+for a device mounted on a wall with y being the vertical axis. This recognizes
+y-acceleration below 800mg, so free fall or changing the orientation of the
+device (y not being the vertical axis and having ~1000mg anymore).
+
+mma8653fc@1d {
+		compatible = "fsl,mma8653fc";
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+		reg = <0x1d>;
+
+		dynamic-range = <2>;
+		ir-freefall-motion-y;
+		irq-threshold = <800>;
+};
+
+Example 3:
+minimal example. lets users read current acceleration values. No polling
+is available.
+
+mma8653fc@1d {
+		compatible = "fsl,mma8653fc";
+		reg = <0x1d>;
+};
diff --git a/drivers/staging/mma8653fc/mma8653fc.c b/drivers/staging/mma8653fc/mma8653fc.c
new file mode 100644
index 0000000..4bd7f99
--- /dev/null
+++ b/drivers/staging/mma8653fc/mma8653fc.c
@@ -0,0 +1,864 @@
+/*
+ * mma8653fc.c - Support for Freescale MMA8653FC 3-axis 10-bit accelerometer
+ *
+ * Copyright (c) 2014 Theobroma Systems Design and Consulting GmbH
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/types.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+
+#define DRV_NAME	"mma8653fc"
+#define MMA8653FC_DEVICE_ID	0x5a
+
+#define MMA8653FC_STATUS	0x00
+
+#define ZYXOW_MASK	0x80
+#define ZYXDR		0x08
+
+#define MMA8653FC_WHO_AM_I	0x0d
+
+#define MMA8653FC_SYSMOD	0x0b
+#define STATE_STANDBY	0x00
+#define STATE_WAKE	0x01
+#define STATE_SLEEP	0x02
+
+#define MMA8450_STATUS_ZXYDR	0x08
+
+/*
+ * 10 bit output data registers
+ * MSB: 7:0 bits 9:2 of data word
+ * LSB: 7:6 bits 1:0 of data word
+ */
+#define MMA8653FC_OUT_X_MSB	0x01
+#define MMA8653FC_OUT_X_LSB	0x02
+#define MMA8653FC_OUT_Y_MSB	0x03
+#define MMA8653FC_OUT_Y_LSB	0x04
+#define MMA8653FC_OUT_Z_MSB	0x05
+#define MMA8653FC_OUT_Z_LSB	0x06
+
+/*
+ * Portrait/Landscape Status
+ */
+#define MMA8653FC_PL_STATUS	0x10
+
+#define NEWLP		0x80
+#define LAPO_HIGH	0x04
+#define LAPO_LOW	0x02
+#define BAFRO		0x01
+
+/*
+ * Portrait/Landscape Configuration
+ */
+#define MMA8653FC_PL_CFG	0x11
+
+#define PL_EN		(1 << 6)
+
+/*
+ * Data calibration registers
+ */
+#define MMA8653FC_OFF_X		0x2f
+#define MMA8653FC_OFF_Y		0x30
+#define MMA8653FC_OFF_Z		0x31
+
+/* 0 to 500 for dts, but -250 to 250 in mg */
+#define DEFAULT_OFF	250
+
+/*
+ * bits 1:0 dynamic range
+ * 00 +/- 2g
+ * 01 +/- 4g
+ * 10 +/- 8g
+ *
+ * HPF_Out bit 4 - data high pass or low pass filtered
+ */
+#define MMA8653FC_XYZ_DATA_CFG	0x0e
+
+#define RANGE_MASK	0x03
+#define RANGE2G		0x00
+#define RANGE4G		0x01
+#define RANGE8G		0x02
+/* values for calculation */
+#define SHIFT_2G	8
+#define INCR_2G		128
+#define SHIFT_4G	7
+#define INCR_4G		64
+#define SHIFT_8G	6
+#define INCR_8G		32
+#define DYN_RANGE_2G	2
+#define DYN_RANGE_4G	4
+#define DYN_RANGE_8G	8
+
+/*
+ * System Control Reg 1
+ */
+#define MMA8653FC_CTRL_REG1	0x2a
+
+#define ACTIVE_BIT	(1 << 0)
+#define ODR_MASK	0x38
+#define ODR_DEFAULT	0x20 /* 50 Hz */
+#define ASLP_RATE_MASK	0xc0
+#define ASLP_RATE_DEFAULT 0x80 /* 6.25 Hz */
+
+/*
+ * Sys Control Reg 2
+ *
+ * auto-sleep enable, software reset
+ */
+#define MMA8653FC_CTRL_REG2	0x2b
+
+#define SLPE		(1 << 2)
+#define SELFTEST	(1 << 7)
+#define SOFT_RESET	(1 << 6)
+
+/*
+ * Interrupt Source
+ */
+#define MMA8653FC_INT_SOURCE	0x0c
+
+#define SRC_ASLP	(1 << 7)
+#define SRC_LNDPRT	(1 << 4)
+#define SRC_FF_MT	(1 << 2)
+#define SRC_DRDY	(1 << 0)
+
+/*
+ * Interrupt Control Register
+ *
+ * default: active low
+ * default push-pull, not open-drain
+ */
+#define MMA8653FC_CTRL_REG3	0x2c
+
+#define WAKE_LNDPRT	(1 << 5)
+#define WAKE_FF_MT	(1 << 3)
+#define IPOL		(1 << 1)
+#define PP_OD		(1 << 0)
+
+/*
+ * Interrupt Enable Register
+ */
+#define MMA8653FC_CTRL_REG4	0x2d
+
+#define INT_EN_ASLP	(1 << 7)
+#define INT_EN_LNDPRT	(1 << 4)
+#define INT_EN_FF_MT	(1 << 2)
+#define INT_EN_DRDY	(1 << 0)
+
+/*
+ * Interrupt Configuration Register
+ * bit value 0 ... INT2 (default)
+ * bit value 1 ... INT1
+ */
+#define MMA8653FC_CTRL_REG5	0x2e
+
+#define INT_CFG_ASLP	(1 << 7)
+#define INT_CFG_LNDPRT	(1 << 4)
+#define INT_CFG_FF_MT	(1 << 2)
+#define INT_CFG_DRDY	(1 << 0)
+
+/*
+ * Freefall / Motion Configuration Register
+ *
+ * Event Latch enable/disable, motion or freefall mode
+ * and event flag enable per axis
+ */
+#define MMA8653FC_FF_MT_CFG	0x15
+
+#define FF_MT_CFG_ELE	(1 << 7)
+#define FF_MT_CFG_OAE	(1 << 6)
+#define FF_MT_CFG_ZEFE	(1 << 5)
+#define FF_MT_CFG_YEFE	(1 << 4)
+#define FF_MT_CFG_XEFE	(1 << 3)
+
+/*
+ * Freefall / Motion Source Register
+ */
+#define MMA8653FC_FF_MT_SRC	0x16
+
+/*
+ * Freefall / Motion Threshold Register
+ *
+ * define motion threshold
+ * 0.063 g/LSB, 127 counts(0x7f) (7 bit from LSB)
+ * range: 0.063g - 8g
+ */
+#define MMA8653FC_FF_MT_THS	0x17
+
+struct axis_triple {
+	s16 x;
+	s16 y;
+	s16 z;
+};
+
+struct mma8653fc_pdata {
+	s8 x_axis_offset;
+	s8 y_axis_offset;
+	s8 z_axis_offset;
+	bool auto_wake_sleep;
+	u32 range;
+	bool int1;
+	bool motion_mode;
+	u8 freefall_motion_thr;
+	bool int_src_data_ready;
+	bool int_src_ff_mt_x;
+	bool int_src_ff_mt_y;
+	bool int_src_ff_mt_z;
+	bool int_src_lndprt;
+	bool int_src_aslp;
+};
+
+struct mma8653fc {
+	struct i2c_client *client;
+	struct mutex mutex;
+	struct mma8653fc_pdata pdata;
+	struct axis_triple saved;
+	char orientation;
+	char bafro;
+	bool standby;
+	int irq;
+	unsigned int_mask;
+	u8 (*read)(struct mma8653fc *, unsigned char);
+	void (*write)(struct mma8653fc *, unsigned char, unsigned char);
+};
+
+/* defaults */
+static const struct mma8653fc_pdata mma8653fc_default_init = {
+	.range = 2,
+	.x_axis_offset = DEFAULT_OFF,
+	.y_axis_offset = DEFAULT_OFF,
+	.z_axis_offset = DEFAULT_OFF,
+	.auto_wake_sleep = false,
+	.int1 = false,
+	.motion_mode = false,
+	.freefall_motion_thr = 5,
+	.int_src_data_ready = false,
+	.int_src_ff_mt_x = false,
+	.int_src_ff_mt_y = false,
+	.int_src_ff_mt_z = false,
+	.int_src_lndprt = false,
+	.int_src_aslp = false,
+};
+
+static void mma8653fc_get_triple(struct mma8653fc *mma)
+{
+	u8 buf[6];
+	u8 status;
+
+	buf[0] = 0;
+
+	status = mma->read(mma, MMA8653FC_STATUS);
+	if (status & ZYXOW_MASK)
+		dev_dbg(&mma->client->dev, "previous read not completed\n");
+
+	buf[0] = mma->read(mma, MMA8653FC_OUT_X_MSB);
+	buf[1] = mma->read(mma, MMA8653FC_OUT_X_LSB);
+	buf[2] = mma->read(mma, MMA8653FC_OUT_Y_MSB);
+	buf[3] = mma->read(mma, MMA8653FC_OUT_Y_LSB);
+	buf[4] = mma->read(mma, MMA8653FC_OUT_Z_MSB);
+	buf[5] = mma->read(mma, MMA8653FC_OUT_Z_LSB);
+
+	mutex_lock(&mma->mutex);
+	/* move from registers to s16 */
+	mma->saved.x = (buf[1] | (buf[0] << 8)) >> 6;
+	mma->saved.y = (buf[3] | (buf[2] << 8)) >> 6;
+	mma->saved.z = (buf[5] | (buf[4] << 8)) >> 6;
+	mma->saved.x = sign_extend32(mma->saved.x, 9);
+	mma->saved.y = sign_extend32(mma->saved.y, 9);
+	mma->saved.z = sign_extend32(mma->saved.z, 9);
+
+	/* calc g, see data sheet and application note */
+	switch (mma->pdata.range) {
+	case DYN_RANGE_2G:
+		mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
+					    INCR_2G) >> SHIFT_2G);
+		mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
+					   INCR_2G) >> SHIFT_2G);
+		mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
+					   INCR_2G) >> SHIFT_2G);
+		break;
+	case DYN_RANGE_4G:
+		mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
+					   INCR_4G) >> SHIFT_4G);
+		mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
+					   INCR_4G) >> SHIFT_4G);
+		mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
+					   INCR_4G) >> SHIFT_4G);
+		break;
+	case DYN_RANGE_8G:
+		mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
+					   INCR_8G) >> SHIFT_8G);
+		mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
+					   INCR_8G) >> SHIFT_8G);
+		mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
+					   INCR_8G) >> SHIFT_8G);
+		break;
+	default:
+		dev_err(&mma->client->dev, "internal data corrupt\n");
+	}
+	mutex_unlock(&mma->mutex);
+}
+
+static void mma8653fc_get_orientation(struct mma8653fc *mma, u8 byte)
+{
+	if ((byte & LAPO_HIGH) && !(LAPO_LOW))
+		mma->orientation = 'r'; /* landscape right */
+	if (!(byte & LAPO_HIGH) && (byte & LAPO_LOW))
+		mma->orientation = 'd'; /* portrait down */
+	if (!(byte & LAPO_HIGH) && !(byte & LAPO_LOW))
+		mma->orientation = 'u'; /* portrait up */
+	if ((byte & LAPO_HIGH) && (byte & LAPO_LOW))
+		mma->orientation = 'l'; /* landscape left */
+
+	if (byte & BAFRO)
+		mma->bafro = 'b'; /* back facing */
+	else
+		mma->bafro = 'f'; /* front facing */
+}
+
+static irqreturn_t mma8653fc_irq(int irq, void *handle)
+{
+	struct mma8653fc *mma = handle;
+	u8 int_src;
+	u8 byte;
+
+	int_src = mma->read(mma, MMA8653FC_INT_SOURCE);
+	if (int_src & SRC_DRDY)
+		/* data ready handle */
+	if (int_src & SRC_FF_MT) {
+		/* freefall/motion change handle */
+		dev_dbg(&mma->client->dev,
+			"freefall or motion change\n");
+		byte = mma->read(mma, MMA8653FC_FF_MT_SRC);
+	}
+	if (int_src & SRC_LNDPRT) {
+		/* landscape/portrait change handle */
+		dev_dbg(&mma->client->dev,
+			"landscape / portrait change\n");
+		byte = mma->read(mma, MMA8653FC_PL_STATUS);
+		mma8653fc_get_orientation(mma, byte);
+	}
+	if (int_src & SRC_ASLP)
+		/* autosleep change handle */
+	mma8653fc_get_triple(mma);
+
+	sysfs_notify(&mma->client->dev.kobj, NULL, "position");
+
+	return IRQ_HANDLED;
+}
+
+static void __mma8653fc_enable(struct mma8653fc *mma)
+{
+	u8 byte;
+
+	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
+	mma->write(mma, MMA8653FC_CTRL_REG1, byte | ACTIVE_BIT);
+}
+
+static void __mma8653fc_disable(struct mma8653fc *mma)
+{
+	u8 byte;
+
+	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
+	mma->write(mma, MMA8653FC_CTRL_REG1, byte & ~ACTIVE_BIT);
+}
+
+static ssize_t mma8653fc_standby_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n", mma->standby);
+}
+
+static ssize_t mma8653fc_standby_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+	unsigned int val;
+	int error;
+
+	error = kstrtouint(buf, 10, &val);
+	if (error)
+		return error;
+
+	mutex_lock(&mma->mutex);
+
+	if (val) {
+		if (!mma->standby)
+			__mma8653fc_disable(mma);
+	} else {
+		if (mma->standby)
+			__mma8653fc_enable(mma);
+	}
+
+	mma->standby = !!val;
+
+	mutex_unlock(&mma->mutex);
+
+	return count;
+}
+
+static DEVICE_ATTR(standby, 0664, mma8653fc_standby_show,
+				  mma8653fc_standby_store);
+
+static ssize_t mma8653fc_currentmode_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+	ssize_t count;
+	u8 byte;
+
+	byte = mma->read(mma, MMA8653FC_SYSMOD);
+	if (byte < 0)
+		return byte;
+
+	switch (byte) {
+	case STATE_STANDBY:
+		count = scnprintf(buf, PAGE_SIZE, "standby\n");
+		break;
+	case STATE_WAKE:
+		count = scnprintf(buf, PAGE_SIZE, "wake\n");
+		break;
+	case STATE_SLEEP:
+		count = scnprintf(buf, PAGE_SIZE, "sleep\n");
+		break;
+	default:
+		count = scnprintf(buf, PAGE_SIZE, "unknown\n");
+	}
+	return count;
+}
+static DEVICE_ATTR(currentmode, S_IRUGO, mma8653fc_currentmode_show, NULL);
+
+static ssize_t mma8653fc_position_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+	ssize_t count;
+	u8 byte;
+
+	if (!mma->irq) {
+		byte = mma->read(mma, MMA8653FC_PL_STATUS);
+		if (byte & NEWLP)
+			mma8653fc_get_orientation(mma, byte);
+
+		byte = mma->read(mma, MMA8653FC_STATUS);
+		if (byte & ZYXDR)
+			mma8653fc_get_triple(mma);
+
+		msleep(20);
+		dev_dbg(&client->dev, "data polled\n");
+	}
+	mutex_lock(&mma->mutex);
+	count = scnprintf(buf, PAGE_SIZE, "%d %d %d %c %c\n",
+			mma->saved.x, mma->saved.y, mma->saved.z,
+			mma->orientation, mma->bafro);
+	mutex_unlock(&mma->mutex);
+
+	return count;
+}
+static DEVICE_ATTR(position, S_IRUGO, mma8653fc_position_show, NULL);
+
+static struct attribute *mma8653fc_attributes[] = {
+	&dev_attr_position.attr,
+	&dev_attr_standby.attr,
+	&dev_attr_currentmode.attr,
+	NULL,
+};
+
+static const struct attribute_group mma8653fc_attr_group = {
+	.attrs = mma8653fc_attributes,
+};
+
+static u8 mma8653fc_read(struct mma8653fc *mma, unsigned char reg)
+{
+	u8 val;
+
+	val = i2c_smbus_read_byte_data(mma->client, reg);
+	if (val < 0) {
+		dev_err(&mma->client->dev,
+			"failed to read %x register\n", reg);
+	}
+	return val;
+}
+
+static void mma8653fc_write(struct mma8653fc *mma, unsigned char reg,
+			   unsigned char val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(mma->client, reg, val);
+	if (ret) {
+		dev_err(&mma->client->dev,
+			"failed to write %x register\n", reg);
+	}
+}
+
+static const struct of_device_id mma8653fc_dt_ids[] = {
+	{ .compatible = "fsl,mma8653fc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mma8653fc_dt_ids);
+
+static const struct mma8653fc_pdata *mma8653fc_probe_dt(struct device *dev)
+{
+	struct mma8653fc_pdata *pdata;
+	struct device_node *node = dev->of_node;
+	const struct of_device_id *match;
+	u32 testu32;
+	s32 tests32;
+
+	if (!node) {
+		dev_err(dev, "no associated DT data\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	match = of_match_device(mma8653fc_dt_ids, dev);
+	if (!match) {
+		dev_err(dev, "unknown device model\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	*pdata = mma8653fc_default_init;
+
+	/* overwrite from dts */
+	testu32 = pdata->x_axis_offset;
+	tests32 = 0;
+	of_property_read_u32(node, "x-offset", &testu32);
+	tests32 = testu32 - DEFAULT_OFF;
+	if ((tests32) && (tests32 <= DEFAULT_OFF) &&
+	    (tests32 >= -DEFAULT_OFF)) {
+		dev_info(dev, "use %dmg offset on X axis\n", tests32);
+		/* calc register value, resolution: 1.96mg */
+		pdata->x_axis_offset = (s8) (tests32 / 2);
+	}
+	testu32 = pdata->y_axis_offset;
+	tests32 = 0;
+	of_property_read_u32(node, "y-offset", &testu32);
+	tests32 = testu32 - DEFAULT_OFF;
+	if ((tests32) && (tests32 <= DEFAULT_OFF) &&
+	    (tests32 >= -DEFAULT_OFF)) {
+		dev_info(dev, "use %dmg offset on Y axis\n", tests32);
+		pdata->y_axis_offset = (s8) (tests32 / 2);
+	}
+	testu32 = pdata->z_axis_offset;
+	tests32 = 0;
+	of_property_read_u32(node, "z-offset", &testu32);
+	tests32 = testu32 - DEFAULT_OFF;
+	if ((tests32) && (tests32 <= DEFAULT_OFF) &&
+	    (tests32 >= -DEFAULT_OFF)) {
+		dev_info(dev, "use %dmg offset on Z axis\n", tests32);
+		pdata->z_axis_offset = (s8) (tests32 / 2);
+	}
+
+	testu32 = 0;
+	of_property_read_u32(node, "dynamic-range", &testu32);
+	if ((testu32) && (testu32 != 2) && (testu32 != 4) && (testu32 != 8)) {
+		dev_warn(dev, "wrong value for full scale range in dtb\n");
+	} else {
+		if (testu32)
+			pdata->range = testu32;
+	}
+
+	if (of_property_read_bool(node, "auto-wake-sleep"))
+		pdata->auto_wake_sleep = true;
+
+	if (of_property_read_bool(node, "int1"))
+		pdata->int1 = true;
+
+	if (of_property_read_bool(node, "motion-mode"))
+		pdata->motion_mode = true;
+
+	if (of_property_read_bool(node, "ir-data-ready"))
+		pdata->int_src_data_ready = true;
+
+	if (of_property_read_bool(node, "ir-freefall-motion-x"))
+		pdata->int_src_ff_mt_x = true;
+
+	if (of_property_read_bool(node, "ir-freefall-motion-y"))
+		pdata->int_src_ff_mt_y = true;
+
+	if (of_property_read_bool(node, "ir-freefall-motion-z"))
+		pdata->int_src_ff_mt_z = true;
+
+	if (of_property_read_bool(node, "ir-auto-wake"))
+		pdata->int_src_aslp = true;
+
+	if (of_property_read_bool(node, "ir-landscape-portrait"))
+		pdata->int_src_lndprt = true;
+
+	testu32 = 0;
+	of_property_read_u32(node, "irq-threshold", &testu32);
+	/* always uses maximum range +/- 8000g, resolution 63mg */
+	if ((testu32 <= 8000) && (testu32 > 0)) {
+		dev_dbg(dev, "use freefall / motion threshold %dmg\n",
+			 testu32);
+		/* calculate register value from mg */
+		pdata->freefall_motion_thr = (testu32 / 63) + 1;
+	}
+
+	return pdata;
+}
+static int mma8653fc_i2c_probe(struct i2c_client *client,
+				       const struct i2c_device_id *id)
+{
+	struct mma8653fc *mma;
+	const struct mma8653fc_pdata *pdata;
+	int err;
+	u8 byte;
+
+	err = i2c_check_functionality(client->adapter,
+			I2C_FUNC_SMBUS_BYTE_DATA);
+	if (!err) {
+		dev_err(&client->dev, "SMBUS Byte Data not Supported\n");
+		return -EIO;
+	}
+
+	mma = devm_kzalloc(&client->dev, sizeof(*mma), GFP_KERNEL);
+	if (!mma)
+		err = -ENOMEM;
+
+	pdata = dev_get_platdata(&client->dev);
+	if (!pdata) {
+		pdata = mma8653fc_probe_dt(&client->dev);
+		if (IS_ERR(pdata)) {
+			err = PTR_ERR(pdata);
+			dev_err(&client->dev, "pdata from DT failed\n");
+			return err;
+		}
+	}
+	mma->pdata = *pdata;
+	pdata = &mma->pdata;
+	mma->client = client;
+	mma->read = &mma8653fc_read;
+	mma->write = &mma8653fc_write;
+
+	mutex_init(&mma->mutex);
+
+	i2c_set_clientdata(client, mma);
+
+	err = sysfs_create_group(&client->dev.kobj, &mma8653fc_attr_group);
+	if (err)
+		return err;
+
+	byte = mma->read(mma, MMA8653FC_WHO_AM_I);
+	if (byte != MMA8653FC_DEVICE_ID) {
+		dev_err(&client->dev, "wrong device for driver\n");
+		return -ENODEV;
+	}
+	dev_info(&client->dev, "loading driver for device id %x\n", byte);
+
+	mma->irq = irq_of_parse_and_map(client->dev.of_node, 0);
+	if (!mma->irq) {
+		dev_err(&client->dev, "Unable to parse or map IRQ\n");
+		goto no_irq;
+	}
+
+	err = irq_set_irq_type(mma->irq, IRQ_TYPE_EDGE_FALLING);
+	if (err) {
+		dev_err(&client->dev, "set_irq_type failed\n");
+		return err;
+	}
+
+	err = devm_request_threaded_irq(&client->dev, mma->irq, NULL,
+					mma8653fc_irq, IRQF_ONESHOT,
+					dev_name(&mma->client->dev), mma);
+	if (err) {
+		dev_err(&client->dev, "irq %d busy?\n", mma->irq);
+		return err;
+	}
+
+	mma->write(mma, MMA8653FC_CTRL_REG2, SOFT_RESET);
+
+	__mma8653fc_disable(mma);
+	mma->standby = true;
+
+	/* enable desired interrupts */
+	mma->orientation = '\0';
+	mma->bafro = '\0';
+	byte = 0;
+	if (pdata->int_src_data_ready) {
+		byte |= INT_EN_DRDY;
+		dev_dbg(&client->dev, "DATA READY interrupt source enabled\n");
+	}
+	if (pdata->int_src_ff_mt_x || pdata->int_src_ff_mt_y ||
+	    pdata->int_src_ff_mt_z) {
+		byte |= INT_EN_FF_MT;
+		dev_dbg(&client->dev, "FF MT interrupt source enabled\n");
+	}
+	if (pdata->int_src_lndprt) {
+		mma->write(mma, MMA8653FC_PL_CFG, PL_EN);
+		byte |= INT_EN_LNDPRT;
+		dev_dbg(&client->dev, "LNDPRT interrupt source enabled\n");
+	}
+	if (pdata->int_src_aslp) {
+		byte |= INT_EN_ASLP;
+		dev_dbg(&client->dev, "ASLP interrupt source enabled\n");
+	}
+	mma->write(mma, MMA8653FC_CTRL_REG4, byte);
+
+	/* force everything to line 1 */
+	if (pdata->int1) {
+		mma->write(mma, MMA8653FC_CTRL_REG5,
+			   (INT_CFG_ASLP | INT_CFG_LNDPRT |
+			   INT_CFG_FF_MT | INT_CFG_DRDY));
+		dev_dbg(&client->dev, "using interrupt line 1\n");
+	}
+no_irq:
+	/* range mode */
+	byte = mma->read(mma, MMA8653FC_XYZ_DATA_CFG);
+	byte &= ~RANGE_MASK;
+	switch (pdata->range) {
+	case DYN_RANGE_2G:
+		byte |= RANGE2G;
+		dev_dbg(&client->dev, "use 2g range\n");
+		break;
+	case DYN_RANGE_4G:
+		byte |= RANGE4G;
+		dev_dbg(&client->dev, "use 4g range\n");
+		break;
+	case DYN_RANGE_8G:
+		byte |= RANGE8G;
+		dev_dbg(&client->dev, "use 8g range\n");
+		break;
+	default:
+		dev_err(&client->dev, "wrong range mode value\n");
+		return -EINVAL;
+	}
+	mma->write(mma, MMA8653FC_XYZ_DATA_CFG, byte);
+
+	/* data calibration offsets */
+	if (pdata->x_axis_offset)
+		mma->write(mma, MMA8653FC_OFF_X, pdata->x_axis_offset);
+	if (pdata->y_axis_offset)
+		mma->write(mma, MMA8653FC_OFF_Y, pdata->y_axis_offset);
+	if (pdata->z_axis_offset)
+		mma->write(mma, MMA8653FC_OFF_Z, pdata->z_axis_offset);
+
+	/* if autosleep, wake on both landscape and motion changes */
+	if (pdata->auto_wake_sleep) {
+		byte = 0;
+		byte |= WAKE_LNDPRT;
+		byte |= WAKE_FF_MT;
+		mma->write(mma, MMA8653FC_CTRL_REG3, byte);
+		mma->write(mma, MMA8653FC_CTRL_REG2, SLPE);
+		dev_dbg(&client->dev, "auto sleep enabled\n");
+	}
+
+	/* data rates */
+	byte = 0;
+	byte = mma->read(mma, MMA8653FC_CTRL_REG1);
+	byte &= ~ODR_MASK;
+	byte |= ODR_DEFAULT;
+	byte &= ~ASLP_RATE_MASK;
+	byte |= ASLP_RATE_DEFAULT;
+	mma->write(mma, MMA8653FC_CTRL_REG1, byte);
+
+	/* freefall / motion config */
+	byte = 0;
+	if (pdata->motion_mode) {
+		byte |= FF_MT_CFG_OAE;
+		dev_dbg(&client->dev, "detect motion instead of freefall\n");
+	}
+	byte |= FF_MT_CFG_ELE;
+	if (pdata->int_src_ff_mt_x)
+		byte |= FF_MT_CFG_XEFE;
+	if (pdata->int_src_ff_mt_y)
+		byte |= FF_MT_CFG_YEFE;
+	if (pdata->int_src_ff_mt_z)
+		byte |= FF_MT_CFG_ZEFE;
+	mma->write(mma, MMA8653FC_FF_MT_CFG, byte);
+
+	if (pdata->freefall_motion_thr) {
+		mma->write(mma, MMA8653FC_FF_MT_THS,
+			   pdata->freefall_motion_thr);
+		/* calculate back to mg */
+		dev_dbg(&client->dev, "threshold set to %dmg\n",
+			 (63 * pdata->freefall_motion_thr) - 1);
+	}
+
+	return 0;
+}
+
+static int mma8653fc_remove(struct i2c_client *client)
+{
+	dev_dbg(&client->dev, "unregistered accelerometer\n");
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mma8653fc_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	__mma8653fc_disable(mma);
+
+	return 0;
+}
+
+static int mma8653fc_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mma8653fc *mma = i2c_get_clientdata(client);
+
+	__mma8653fc_enable(mma);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mma8653fc_pm_ops, mma8653fc_suspend, mma8653fc_resume);
+#define MMA8653FC_PM_OPS (&mma8653fc_pm_ops)
+#else
+#define MMA8653FC_PM_OPS NULL
+#endif
+
+static const struct i2c_device_id mma8653fc_id[] = {
+	{ DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma8653fc_id);
+
+static struct i2c_driver mma8653fc_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = mma8653fc_dt_ids,
+		.pm = MMA8653FC_PM_OPS,
+	},
+	.probe    = mma8653fc_i2c_probe,
+	.remove   = mma8653fc_remove,
+	.id_table = mma8653fc_id,
+};
+
+module_i2c_driver(mma8653fc_driver);
+
+MODULE_AUTHOR("Martin Kepplinger <martin.kepplinger@theobroma-systems.com");
+MODULE_DESCRIPTION("Freescale's MMA8653FC Three-Axis Accelerometer I2C Driver");
+MODULE_LICENSE("GPL");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH V7] Allow compaction of unevictable pages
From: Rik van Riel @ 2015-03-20 14:47 UTC (permalink / raw)
  To: Eric B Munson, Andrew Morton
  Cc: Vlastimil Babka, Thomas Gleixner, Christoph Lameter,
	Peter Zijlstra, Mel Gorman, David Rientjes, Michal Hocko,
	linux-doc, linux-rt-users, linux-mm, linux-api, linux-kernel
In-Reply-To: <1426859390-10974-1-git-send-email-emunson@akamai.com>

On 03/20/2015 09:49 AM, Eric B Munson wrote:
> Currently, pages which are marked as unevictable are protected from
> compaction, but not from other types of migration.  The POSIX real time
> extension explicitly states that mlock() will prevent a major page
> fault, but the spirit of this is that mlock() should give a process the
> ability to control sources of latency, including minor page faults.
> However, the mlock manpage only explicitly says that a locked page will
> not be written to swap and this can cause some confusion.  The
> compaction code today does not give a developer who wants to avoid swap
> but wants to have large contiguous areas available any method to achieve
> this state.  This patch introduces a sysctl for controlling compaction
> behavior with respect to the unevictable lru.  Users that demand no page
> faults after a page is present can set compact_unevictable_allowed to 0
> and users who need the large contiguous areas can enable compaction on
> locked memory by leaving the default value of 1.
> 
> To illustrate this problem I wrote a quick test program that mmaps a
> large number of 1MB files filled with random data.  These maps are
> created locked and read only.  Then every other mmap is unmapped and I
> attempt to allocate huge pages to the static huge page pool.  When the
> compact_unevictable_allowed sysctl is 0, I cannot allocate hugepages
> after fragmenting memory.  When the value is set to 1, allocations
> succeed.
> 
> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Acked-by: Michal Hocko <mhocko@suse.cz>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: Christoph Lameter <cl@linux.com>
> Acked-by: David Rientjes <rientjes@google.com>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

^ permalink raw reply

* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Alexander Shishkin @ 2015-03-20 14:53 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, Pratik Patel,
	peter.lachner, norbert.schulz, keven.boell, yann.fouassier,
	laurent.fert, linux-api
In-Reply-To: <CANLsYkwithjLgtztf8aBDRv-QL2VGOwc4XZ=1sGBSHg4LiymBg@mail.gmail.com>

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> As promised I worked on a prototype that connects the coresight-stm
> driver with the generic STM interface you have suggested.  Things work
> quite well and aside from the enhancement related to the ioctl() and
> private member as discussed above, we should move ahead with this.
>
> I will send out a new version of the coresight-stm driver as soon as I
> see your patches with those changes.

Actually, instread of a private member I'd simply pass struct stm_data
pointer to the callback (like we do with other callbacks) and the
private data would be in the structure that embeds this struct stm_data,
so that you can get to it using container_of():

struct my_stm {
       struct stm_data	data;
       void		*my_stuff;
       ...
};

...

long my_ioctl(struct stm_data *data, unsigned int cmd, unsigned long
	      arg)
{
        struct my_stm *mine = container_of(data, struct my_stm, data);

...

Would this work for you? I'm otherwise ready to send the second version
of my patchset.

Regards,
--
Alex

^ permalink raw reply

* Re: [PATCH v0 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Mathieu Poirier @ 2015-03-20 15:19 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Greg Kroah-Hartman,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pratik Patel, peter.lachner-ral2JQCrhuEAvxtiuMwx3w,
	norbert.schulz-ral2JQCrhuEAvxtiuMwx3w,
	keven.boell-ral2JQCrhuEAvxtiuMwx3w,
	yann.fouassier-ral2JQCrhuEAvxtiuMwx3w,
	laurent.fert-ral2JQCrhuEAvxtiuMwx3w,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87k2ybd8jl.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>

On 20 March 2015 at 08:53, Alexander Shishkin
<alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
>
>> As promised I worked on a prototype that connects the coresight-stm
>> driver with the generic STM interface you have suggested.  Things work
>> quite well and aside from the enhancement related to the ioctl() and
>> private member as discussed above, we should move ahead with this.
>>
>> I will send out a new version of the coresight-stm driver as soon as I
>> see your patches with those changes.
>
> Actually, instread of a private member I'd simply pass struct stm_data
> pointer to the callback (like we do with other callbacks) and the
> private data would be in the structure that embeds this struct stm_data,
> so that you can get to it using container_of():
>
> struct my_stm {
>        struct stm_data  data;
>        void             *my_stuff;
>        ...
> };
>
> ...
>
> long my_ioctl(struct stm_data *data, unsigned int cmd, unsigned long
>               arg)
> {
>         struct my_stm *mine = container_of(data, struct my_stm, data);
>
> ...
>
> Would this work for you?

That should be just fine yes.

> I'm otherwise ready to send the second version
> of my patchset.

Cool

>
> Regards,
> --
> Alex

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox