[PATCH] Tracing events with GPIOs

[PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

This patch implements a new tracing mechanism based on kprobes and using GPIO.
Debugging with GPIO is very common in the embedded world. At least for those of us
fortunate enough to have an oscilloscope or a logic analyzer on their bench...
This is especially true if the issue results of a hardware/sofware interaction.

Typical use cases are :
* mixed software/hardware debugging. For example when the software detects a
  situation of interest (typically an error) it toggles a GPIO to trigger the
  oscilloscope acquisition.
* direct latency/duration measurements.

examples:
To trig the oscilloscope whenever a mmc command error:
  echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
  echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
---
 kernel/trace/Kconfig        |  12 ++++
 kernel/trace/trace_kprobe.c | 167 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 177 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 015f85a..4228768 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -420,6 +420,18 @@ config KPROBE_EVENT
 	  This option is also required by perf-probe subcommand of perf tools.
 	  If you want to use perf tools, this option is strongly recommended.
 
+config KPROBE_EVENT_GPIO
+	depends on KPROBE_EVENT
+	depends on GPIOLIB
+	bool "Enable kprobes-based tracing of events via GPIO"
+	default n
+	help
+		This allows the user to use GPIOs as a tracing tool.The primary
+		purpose of this option is to allow the user to trigger an
+		oscilloscope on software events.
+		The GPIO can be set, cleared, toggled, or pulsed when the event
+		is hit.
+
 config UPROBE_EVENT
 	bool "Enable uprobes-based dynamic events"
 	depends on ARCH_SUPPORTS_UPROBES
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index dae9541..5d297f5 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -19,6 +19,7 @@
 
 #include <linux/module.h>
 #include <linux/uaccess.h>
+#include <linux/gpio.h>
 
 #include "trace_probe.h"
 
@@ -27,6 +28,33 @@
 /**
  * Kprobe event core functions
  */
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+#define TP_GPIO_UNDEFINED      0
+#define TP_GPIO_CMD_SET                1
+#define TP_GPIO_CMD_CLEAR      2
+#define TP_GPIO_CMD_TOGGLE     3
+#define TP_GPIO_CMD_PULSE      4
+#define TP_GPIO_CMD_PULSE_HIGH 5
+#define TP_GPIO_CMD_PULSE_LOW  6
+
+static const struct {
+	const char *name;
+	int action;
+} gpio_actions[] = {
+	{"gpioset", TP_GPIO_CMD_SET},
+	{"gpioclear", TP_GPIO_CMD_CLEAR},
+	{"gpiotoggle", TP_GPIO_CMD_TOGGLE},
+	{"gpiopulse", TP_GPIO_CMD_PULSE},
+	{"gpiopulsehigh", TP_GPIO_CMD_PULSE_HIGH},
+	{"gpiopulselow", TP_GPIO_CMD_PULSE_LOW},
+};
+
+struct gpio_trace {
+	int gpio;
+	int action;
+};
+#endif
+
 struct trace_probe {
 	struct list_head	list;
 	struct kretprobe	rp;	/* Use rp.kp for kprobe use */
@@ -38,6 +66,9 @@ struct trace_probe {
 	struct list_head	files;
 	ssize_t			size;		/* trace entry size */
 	unsigned int		nr_args;
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+	struct gpio_trace               gpio_trace;
+#endif
 	struct probe_arg	args[];
 };
 
@@ -164,10 +195,24 @@ error:
 	return ERR_PTR(ret);
 }
 
+static struct trace_probe *find_gpio_probe(int gpio)
+{
+	struct trace_probe *tp;
+
+	list_for_each_entry(tp, &probe_list, list)
+	if ((tp->gpio_trace.action) && (tp->gpio_trace.gpio == gpio))
+		return tp;
+	return NULL;
+}
+
 static void free_trace_probe(struct trace_probe *tp)
 {
 	int i;
 
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+	if (tp->gpio_trace.action && !find_gpio_probe(tp->gpio_trace.gpio))
+		gpio_free(tp->gpio_trace.gpio);
+#endif
 	for (i = 0; i < tp->nr_args; i++)
 		traceprobe_free_probe_arg(&tp->args[i]);
 
@@ -435,8 +480,14 @@ static int create_trace_probe(int argc, char **argv)
 {
 	/*
 	 * Argument syntax:
-	 *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
-	 *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
+	 *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR
+	 *                [GPIOACTION@GPIONUM] [FETCHARGS]
+	 *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0]
+	 *                   [GPIOACTION@GPIONUM] [FETCHARGS]
+	 * Gpio tracing:
+	 *   gpio action can be : gpioset, gpioclear, gpiotoggle, gpiopulse,
+	 *                        gpiopulsehigh and gpiopulselow
+	 *   gpionum is the id of the gpio
 	 * Fetch args:
 	 *  $retval	: fetch return value
 	 *  $stack	: fetch stack address
@@ -459,6 +510,9 @@ static int create_trace_probe(int argc, char **argv)
 	unsigned long offset = 0;
 	void *addr = NULL;
 	char buf[MAX_EVENT_NAME_LEN];
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+	int len;
+#endif
 
 	/* argc must be >= 1 */
 	if (argv[0][0] == 'p')
@@ -541,6 +595,7 @@ static int create_trace_probe(int argc, char **argv)
 			return -EINVAL;
 		}
 	}
+
 	argc -= 2; argv += 2;
 
 	/* setup a probe */
@@ -562,6 +617,66 @@ static int create_trace_probe(int argc, char **argv)
 		return PTR_ERR(tp);
 	}
 
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+	/* parse the optionnal gpio action argument */
+	for (i = 0; argc && (i < ARRAY_SIZE(gpio_actions)); i++) {
+		len = strlen(gpio_actions[i].name);
+		if (strncmp(argv[0], gpio_actions[i].name, len) == 0)
+			break;
+	}
+
+	if (argc && (i < ARRAY_SIZE(gpio_actions))) {
+		int gpio;
+		unsigned long ul;
+		int gpio_requested = 0;
+		ret = -EINVAL;
+		if (argv[0][len] != '@') {
+			pr_info("Syntax error. wrong gpio syntax\n");
+			goto error;
+		}
+
+		if (kstrtoul(&argv[0][len+1], 0, &ul)) {
+			pr_info("Failed to parse gpio number.\n");
+			goto error;
+		}
+		gpio = ul;
+
+		if (!gpio_is_valid(gpio)) {
+			pr_info("gpio %d is not valid.\n", gpio);
+			goto error;
+		}
+
+		if (gpio_cansleep(gpio))
+			pr_info("gpio %d can sleep. This may break your"
+				"kernel!!!!!!\n", gpio);
+
+		if (!find_gpio_probe(gpio)) {
+			ret = gpio_request(gpio, event);
+			if (ret) {
+				pr_info("can't request gpio %d.\n", gpio);
+				goto error;
+			}
+			gpio_requested = 1;
+		}
+
+		if ((gpio_actions[i].action == TP_GPIO_CMD_CLEAR) ||
+			(gpio_actions[i].action == TP_GPIO_CMD_PULSE_LOW))
+			ret = gpio_direction_output(gpio, 1);
+		else
+			ret = gpio_direction_output(gpio, 0);
+		if (ret) {
+			pr_info("can't make gpio %d an output.\n", gpio);
+			if (gpio_requested)
+				gpio_free(gpio);
+			goto error;
+		}
+
+		tp->gpio_trace.gpio = gpio;
+		tp->gpio_trace.action = gpio_actions[i].action;
+		argc -= 1; argv += 1;
+	}
+#endif
+
 	/* parse arguments */
 	ret = 0;
 	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
@@ -1145,12 +1260,54 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
 }
 #endif	/* CONFIG_PERF_EVENTS */
 
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+static __kprobes void kprobe_gpio_take_action(int gpio, int action)
+{
+	int val;
+	switch (action) {
+	case TP_GPIO_CMD_PULSE_HIGH:
+		gpio_set_value(gpio, 1);
+		/* intentionnaly don't break here */
+	case TP_GPIO_CMD_CLEAR:
+		gpio_set_value(gpio, 0);
+	break;
+
+	case TP_GPIO_CMD_PULSE_LOW:
+		gpio_set_value(gpio, 0);
+		/* intentionnaly don't break here */
+	case TP_GPIO_CMD_SET:
+		gpio_set_value(gpio, 1);
+	break;
+
+	case TP_GPIO_CMD_TOGGLE:
+		val = gpio_get_value(gpio);
+		gpio_set_value(gpio, val ? 0 : 1);
+	break;
+
+	case TP_GPIO_CMD_PULSE:
+		val = gpio_get_value(gpio);
+		gpio_set_value(gpio, val ? 0 : 1);
+		gpio_set_value(gpio, val);
+	break;
+	}
+}
+
 /*
  * called by perf_trace_init() or __ftrace_set_clr_event() under event_mutex.
  *
  * kprobe_trace_self_tests_init() does enable_trace_probe/disable_trace_probe
  * lockless, but we can't race with this __init function.
  */
+/* Kprobe gpio handler */
+static inline __kprobes void kprobe_gpio_func(struct trace_probe *tp,
+					      struct pt_regs *regs)
+{
+	if (tp->gpio_trace.action)
+		kprobe_gpio_take_action(tp->gpio_trace.gpio,
+					tp->gpio_trace.action);
+}
+#endif /* CONFIG_KPROBE_EVENT_GPIO */
+
 static __kprobes
 int kprobe_register(struct ftrace_event_call *event,
 		    enum trace_reg type, void *data)
@@ -1186,6 +1343,9 @@ int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
 
 	tp->nhit++;
 
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+	kprobe_gpio_func(tp, regs);
+#endif
 	if (tp->flags & TP_FLAG_TRACE)
 		kprobe_trace_func(tp, regs);
 #ifdef CONFIG_PERF_EVENTS
@@ -1202,6 +1362,9 @@ int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
 
 	tp->nhit++;
 
+#ifdef CONFIG_KPROBE_EVENT_GPIO
+	kprobe_gpio_func(tp, regs);
+#endif
 	if (tp->flags & TP_FLAG_TRACE)
 		kretprobe_trace_func(tp, ri, regs);
 #ifdef CONFIG_PERF_EVENTS
-- 
1.8.3.1

Re: [PATCH] Tracing events with GPIOs

[Masami Hiramatsu]

(2013/12/17 9:22), Jean-Jacques Hiblot wrote:
> This patch implements a new tracing mechanism based on kprobes and using GPIO.
> Debugging with GPIO is very common in the embedded world. At least for those of us
> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
> This is especially true if the issue results of a hardware/sofware interaction.
> 
> Typical use cases are :
> * mixed software/hardware debugging. For example when the software detects a
>   situation of interest (typically an error) it toggles a GPIO to trigger the
>   oscilloscope acquisition.
> * direct latency/duration measurements.

Ah, it's interesting to me :)

And I think this feature should be built on the event triggers which
Tom is working on, instead of kprobes directly, because it allows
you to take gpio actions on normal tracepoints, and uprobes too :)

Thank you!

> 
> examples:
> To trig the oscilloscope whenever a mmc command error:
>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> ---
>  kernel/trace/Kconfig        |  12 ++++
>  kernel/trace/trace_kprobe.c | 167 +++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 177 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 015f85a..4228768 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -420,6 +420,18 @@ config KPROBE_EVENT
>  	  This option is also required by perf-probe subcommand of perf tools.
>  	  If you want to use perf tools, this option is strongly recommended.
>  
> +config KPROBE_EVENT_GPIO
> +	depends on KPROBE_EVENT
> +	depends on GPIOLIB
> +	bool "Enable kprobes-based tracing of events via GPIO"
> +	default n
> +	help
> +		This allows the user to use GPIOs as a tracing tool.The primary
> +		purpose of this option is to allow the user to trigger an
> +		oscilloscope on software events.
> +		The GPIO can be set, cleared, toggled, or pulsed when the event
> +		is hit.
> +
>  config UPROBE_EVENT
>  	bool "Enable uprobes-based dynamic events"
>  	depends on ARCH_SUPPORTS_UPROBES
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index dae9541..5d297f5 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -19,6 +19,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/uaccess.h>
> +#include <linux/gpio.h>
>  
>  #include "trace_probe.h"
>  
> @@ -27,6 +28,33 @@
>  /**
>   * Kprobe event core functions
>   */
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +#define TP_GPIO_UNDEFINED      0
> +#define TP_GPIO_CMD_SET                1
> +#define TP_GPIO_CMD_CLEAR      2
> +#define TP_GPIO_CMD_TOGGLE     3
> +#define TP_GPIO_CMD_PULSE      4
> +#define TP_GPIO_CMD_PULSE_HIGH 5
> +#define TP_GPIO_CMD_PULSE_LOW  6
> +
> +static const struct {
> +	const char *name;
> +	int action;
> +} gpio_actions[] = {
> +	{"gpioset", TP_GPIO_CMD_SET},
> +	{"gpioclear", TP_GPIO_CMD_CLEAR},
> +	{"gpiotoggle", TP_GPIO_CMD_TOGGLE},
> +	{"gpiopulse", TP_GPIO_CMD_PULSE},
> +	{"gpiopulsehigh", TP_GPIO_CMD_PULSE_HIGH},
> +	{"gpiopulselow", TP_GPIO_CMD_PULSE_LOW},
> +};
> +
> +struct gpio_trace {
> +	int gpio;
> +	int action;
> +};
> +#endif
> +
>  struct trace_probe {
>  	struct list_head	list;
>  	struct kretprobe	rp;	/* Use rp.kp for kprobe use */
> @@ -38,6 +66,9 @@ struct trace_probe {
>  	struct list_head	files;
>  	ssize_t			size;		/* trace entry size */
>  	unsigned int		nr_args;
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +	struct gpio_trace               gpio_trace;
> +#endif
>  	struct probe_arg	args[];
>  };
>  
> @@ -164,10 +195,24 @@ error:
>  	return ERR_PTR(ret);
>  }
>  
> +static struct trace_probe *find_gpio_probe(int gpio)
> +{
> +	struct trace_probe *tp;
> +
> +	list_for_each_entry(tp, &probe_list, list)
> +	if ((tp->gpio_trace.action) && (tp->gpio_trace.gpio == gpio))
> +		return tp;
> +	return NULL;
> +}
> +
>  static void free_trace_probe(struct trace_probe *tp)
>  {
>  	int i;
>  
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +	if (tp->gpio_trace.action && !find_gpio_probe(tp->gpio_trace.gpio))
> +		gpio_free(tp->gpio_trace.gpio);
> +#endif
>  	for (i = 0; i < tp->nr_args; i++)
>  		traceprobe_free_probe_arg(&tp->args[i]);
>  
> @@ -435,8 +480,14 @@ static int create_trace_probe(int argc, char **argv)
>  {
>  	/*
>  	 * Argument syntax:
> -	 *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
> -	 *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
> +	 *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR
> +	 *                [GPIOACTION@GPIONUM] [FETCHARGS]
> +	 *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0]
> +	 *                   [GPIOACTION@GPIONUM] [FETCHARGS]
> +	 * Gpio tracing:
> +	 *   gpio action can be : gpioset, gpioclear, gpiotoggle, gpiopulse,
> +	 *                        gpiopulsehigh and gpiopulselow
> +	 *   gpionum is the id of the gpio
>  	 * Fetch args:
>  	 *  $retval	: fetch return value
>  	 *  $stack	: fetch stack address
> @@ -459,6 +510,9 @@ static int create_trace_probe(int argc, char **argv)
>  	unsigned long offset = 0;
>  	void *addr = NULL;
>  	char buf[MAX_EVENT_NAME_LEN];
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +	int len;
> +#endif
>  
>  	/* argc must be >= 1 */
>  	if (argv[0][0] == 'p')
> @@ -541,6 +595,7 @@ static int create_trace_probe(int argc, char **argv)
>  			return -EINVAL;
>  		}
>  	}
> +
>  	argc -= 2; argv += 2;
>  
>  	/* setup a probe */
> @@ -562,6 +617,66 @@ static int create_trace_probe(int argc, char **argv)
>  		return PTR_ERR(tp);
>  	}
>  
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +	/* parse the optionnal gpio action argument */
> +	for (i = 0; argc && (i < ARRAY_SIZE(gpio_actions)); i++) {
> +		len = strlen(gpio_actions[i].name);
> +		if (strncmp(argv[0], gpio_actions[i].name, len) == 0)
> +			break;
> +	}
> +
> +	if (argc && (i < ARRAY_SIZE(gpio_actions))) {
> +		int gpio;
> +		unsigned long ul;
> +		int gpio_requested = 0;
> +		ret = -EINVAL;
> +		if (argv[0][len] != '@') {
> +			pr_info("Syntax error. wrong gpio syntax\n");
> +			goto error;
> +		}
> +
> +		if (kstrtoul(&argv[0][len+1], 0, &ul)) {
> +			pr_info("Failed to parse gpio number.\n");
> +			goto error;
> +		}
> +		gpio = ul;
> +
> +		if (!gpio_is_valid(gpio)) {
> +			pr_info("gpio %d is not valid.\n", gpio);
> +			goto error;
> +		}
> +
> +		if (gpio_cansleep(gpio))
> +			pr_info("gpio %d can sleep. This may break your"
> +				"kernel!!!!!!\n", gpio);
> +
> +		if (!find_gpio_probe(gpio)) {
> +			ret = gpio_request(gpio, event);
> +			if (ret) {
> +				pr_info("can't request gpio %d.\n", gpio);
> +				goto error;
> +			}
> +			gpio_requested = 1;
> +		}
> +
> +		if ((gpio_actions[i].action == TP_GPIO_CMD_CLEAR) ||
> +			(gpio_actions[i].action == TP_GPIO_CMD_PULSE_LOW))
> +			ret = gpio_direction_output(gpio, 1);
> +		else
> +			ret = gpio_direction_output(gpio, 0);
> +		if (ret) {
> +			pr_info("can't make gpio %d an output.\n", gpio);
> +			if (gpio_requested)
> +				gpio_free(gpio);
> +			goto error;
> +		}
> +
> +		tp->gpio_trace.gpio = gpio;
> +		tp->gpio_trace.action = gpio_actions[i].action;
> +		argc -= 1; argv += 1;
> +	}
> +#endif
> +
>  	/* parse arguments */
>  	ret = 0;
>  	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
> @@ -1145,12 +1260,54 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
>  }
>  #endif	/* CONFIG_PERF_EVENTS */
>  
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +static __kprobes void kprobe_gpio_take_action(int gpio, int action)
> +{
> +	int val;
> +	switch (action) {
> +	case TP_GPIO_CMD_PULSE_HIGH:
> +		gpio_set_value(gpio, 1);
> +		/* intentionnaly don't break here */
> +	case TP_GPIO_CMD_CLEAR:
> +		gpio_set_value(gpio, 0);
> +	break;
> +
> +	case TP_GPIO_CMD_PULSE_LOW:
> +		gpio_set_value(gpio, 0);
> +		/* intentionnaly don't break here */
> +	case TP_GPIO_CMD_SET:
> +		gpio_set_value(gpio, 1);
> +	break;
> +
> +	case TP_GPIO_CMD_TOGGLE:
> +		val = gpio_get_value(gpio);
> +		gpio_set_value(gpio, val ? 0 : 1);
> +	break;
> +
> +	case TP_GPIO_CMD_PULSE:
> +		val = gpio_get_value(gpio);
> +		gpio_set_value(gpio, val ? 0 : 1);
> +		gpio_set_value(gpio, val);
> +	break;
> +	}
> +}
> +
>  /*
>   * called by perf_trace_init() or __ftrace_set_clr_event() under event_mutex.
>   *
>   * kprobe_trace_self_tests_init() does enable_trace_probe/disable_trace_probe
>   * lockless, but we can't race with this __init function.
>   */
> +/* Kprobe gpio handler */
> +static inline __kprobes void kprobe_gpio_func(struct trace_probe *tp,
> +					      struct pt_regs *regs)
> +{
> +	if (tp->gpio_trace.action)
> +		kprobe_gpio_take_action(tp->gpio_trace.gpio,
> +					tp->gpio_trace.action);
> +}
> +#endif /* CONFIG_KPROBE_EVENT_GPIO */
> +
>  static __kprobes
>  int kprobe_register(struct ftrace_event_call *event,
>  		    enum trace_reg type, void *data)
> @@ -1186,6 +1343,9 @@ int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
>  
>  	tp->nhit++;
>  
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +	kprobe_gpio_func(tp, regs);
> +#endif
>  	if (tp->flags & TP_FLAG_TRACE)
>  		kprobe_trace_func(tp, regs);
>  #ifdef CONFIG_PERF_EVENTS
> @@ -1202,6 +1362,9 @@ int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
>  
>  	tp->nhit++;
>  
> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> +	kprobe_gpio_func(tp, regs);
> +#endif
>  	if (tp->flags & TP_FLAG_TRACE)
>  		kretprobe_trace_func(tp, ri, regs);
>  #ifdef CONFIG_PERF_EVENTS
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

2013/12/17 Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>:
> (2013/12/17 9:22), Jean-Jacques Hiblot wrote:
>> This patch implements a new tracing mechanism based on kprobes and using GPIO.
>> Debugging with GPIO is very common in the embedded world. At least for those of us
>> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
>> This is especially true if the issue results of a hardware/sofware interaction.
>>
>> Typical use cases are :
>> * mixed software/hardware debugging. For example when the software detects a
>>   situation of interest (typically an error) it toggles a GPIO to trigger the
>>   oscilloscope acquisition.
>> * direct latency/duration measurements.
>
> Ah, it's interesting to me :)
>
> And I think this feature should be built on the event triggers which
> Tom is working on, instead of kprobes directly, because it allows
> you to take gpio actions on normal tracepoints, and uprobes too :)
>

I'll make a version that uses this framework then. When it's ready
I'll make some measurements to check the overhead of both solutions.

Tom,
is git://git.yoctoproject.org/linux-yocto-contrib
tzanussi/event-triggers-v11 the right starting point ?

Thanks,

Jean-Jacques

> Thank you!
>
>>
>> examples:
>> To trig the oscilloscope whenever a mmc command error:
>>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> ---
>>  kernel/trace/Kconfig        |  12 ++++
>>  kernel/trace/trace_kprobe.c | 167 +++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 177 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
>> index 015f85a..4228768 100644
>> --- a/kernel/trace/Kconfig
>> +++ b/kernel/trace/Kconfig
>> @@ -420,6 +420,18 @@ config KPROBE_EVENT
>>         This option is also required by perf-probe subcommand of perf tools.
>>         If you want to use perf tools, this option is strongly recommended.
>>
>> +config KPROBE_EVENT_GPIO
>> +     depends on KPROBE_EVENT
>> +     depends on GPIOLIB
>> +     bool "Enable kprobes-based tracing of events via GPIO"
>> +     default n
>> +     help
>> +             This allows the user to use GPIOs as a tracing tool.The primary
>> +             purpose of this option is to allow the user to trigger an
>> +             oscilloscope on software events.
>> +             The GPIO can be set, cleared, toggled, or pulsed when the event
>> +             is hit.
>> +
>>  config UPROBE_EVENT
>>       bool "Enable uprobes-based dynamic events"
>>       depends on ARCH_SUPPORTS_UPROBES
>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
>> index dae9541..5d297f5 100644
>> --- a/kernel/trace/trace_kprobe.c
>> +++ b/kernel/trace/trace_kprobe.c
>> @@ -19,6 +19,7 @@
>>
>>  #include <linux/module.h>
>>  #include <linux/uaccess.h>
>> +#include <linux/gpio.h>
>>
>>  #include "trace_probe.h"
>>
>> @@ -27,6 +28,33 @@
>>  /**
>>   * Kprobe event core functions
>>   */
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +#define TP_GPIO_UNDEFINED      0
>> +#define TP_GPIO_CMD_SET                1
>> +#define TP_GPIO_CMD_CLEAR      2
>> +#define TP_GPIO_CMD_TOGGLE     3
>> +#define TP_GPIO_CMD_PULSE      4
>> +#define TP_GPIO_CMD_PULSE_HIGH 5
>> +#define TP_GPIO_CMD_PULSE_LOW  6
>> +
>> +static const struct {
>> +     const char *name;
>> +     int action;
>> +} gpio_actions[] = {
>> +     {"gpioset", TP_GPIO_CMD_SET},
>> +     {"gpioclear", TP_GPIO_CMD_CLEAR},
>> +     {"gpiotoggle", TP_GPIO_CMD_TOGGLE},
>> +     {"gpiopulse", TP_GPIO_CMD_PULSE},
>> +     {"gpiopulsehigh", TP_GPIO_CMD_PULSE_HIGH},
>> +     {"gpiopulselow", TP_GPIO_CMD_PULSE_LOW},
>> +};
>> +
>> +struct gpio_trace {
>> +     int gpio;
>> +     int action;
>> +};
>> +#endif
>> +
>>  struct trace_probe {
>>       struct list_head        list;
>>       struct kretprobe        rp;     /* Use rp.kp for kprobe use */
>> @@ -38,6 +66,9 @@ struct trace_probe {
>>       struct list_head        files;
>>       ssize_t                 size;           /* trace entry size */
>>       unsigned int            nr_args;
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +     struct gpio_trace               gpio_trace;
>> +#endif
>>       struct probe_arg        args[];
>>  };
>>
>> @@ -164,10 +195,24 @@ error:
>>       return ERR_PTR(ret);
>>  }
>>
>> +static struct trace_probe *find_gpio_probe(int gpio)
>> +{
>> +     struct trace_probe *tp;
>> +
>> +     list_for_each_entry(tp, &probe_list, list)
>> +     if ((tp->gpio_trace.action) && (tp->gpio_trace.gpio == gpio))
>> +             return tp;
>> +     return NULL;
>> +}
>> +
>>  static void free_trace_probe(struct trace_probe *tp)
>>  {
>>       int i;
>>
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +     if (tp->gpio_trace.action && !find_gpio_probe(tp->gpio_trace.gpio))
>> +             gpio_free(tp->gpio_trace.gpio);
>> +#endif
>>       for (i = 0; i < tp->nr_args; i++)
>>               traceprobe_free_probe_arg(&tp->args[i]);
>>
>> @@ -435,8 +480,14 @@ static int create_trace_probe(int argc, char **argv)
>>  {
>>       /*
>>        * Argument syntax:
>> -      *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
>> -      *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
>> +      *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR
>> +      *                [GPIOACTION@GPIONUM] [FETCHARGS]
>> +      *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0]
>> +      *                   [GPIOACTION@GPIONUM] [FETCHARGS]
>> +      * Gpio tracing:
>> +      *   gpio action can be : gpioset, gpioclear, gpiotoggle, gpiopulse,
>> +      *                        gpiopulsehigh and gpiopulselow
>> +      *   gpionum is the id of the gpio
>>        * Fetch args:
>>        *  $retval     : fetch return value
>>        *  $stack      : fetch stack address
>> @@ -459,6 +510,9 @@ static int create_trace_probe(int argc, char **argv)
>>       unsigned long offset = 0;
>>       void *addr = NULL;
>>       char buf[MAX_EVENT_NAME_LEN];
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +     int len;
>> +#endif
>>
>>       /* argc must be >= 1 */
>>       if (argv[0][0] == 'p')
>> @@ -541,6 +595,7 @@ static int create_trace_probe(int argc, char **argv)
>>                       return -EINVAL;
>>               }
>>       }
>> +
>>       argc -= 2; argv += 2;
>>
>>       /* setup a probe */
>> @@ -562,6 +617,66 @@ static int create_trace_probe(int argc, char **argv)
>>               return PTR_ERR(tp);
>>       }
>>
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +     /* parse the optionnal gpio action argument */
>> +     for (i = 0; argc && (i < ARRAY_SIZE(gpio_actions)); i++) {
>> +             len = strlen(gpio_actions[i].name);
>> +             if (strncmp(argv[0], gpio_actions[i].name, len) == 0)
>> +                     break;
>> +     }
>> +
>> +     if (argc && (i < ARRAY_SIZE(gpio_actions))) {
>> +             int gpio;
>> +             unsigned long ul;
>> +             int gpio_requested = 0;
>> +             ret = -EINVAL;
>> +             if (argv[0][len] != '@') {
>> +                     pr_info("Syntax error. wrong gpio syntax\n");
>> +                     goto error;
>> +             }
>> +
>> +             if (kstrtoul(&argv[0][len+1], 0, &ul)) {
>> +                     pr_info("Failed to parse gpio number.\n");
>> +                     goto error;
>> +             }
>> +             gpio = ul;
>> +
>> +             if (!gpio_is_valid(gpio)) {
>> +                     pr_info("gpio %d is not valid.\n", gpio);
>> +                     goto error;
>> +             }
>> +
>> +             if (gpio_cansleep(gpio))
>> +                     pr_info("gpio %d can sleep. This may break your"
>> +                             "kernel!!!!!!\n", gpio);
>> +
>> +             if (!find_gpio_probe(gpio)) {
>> +                     ret = gpio_request(gpio, event);
>> +                     if (ret) {
>> +                             pr_info("can't request gpio %d.\n", gpio);
>> +                             goto error;
>> +                     }
>> +                     gpio_requested = 1;
>> +             }
>> +
>> +             if ((gpio_actions[i].action == TP_GPIO_CMD_CLEAR) ||
>> +                     (gpio_actions[i].action == TP_GPIO_CMD_PULSE_LOW))
>> +                     ret = gpio_direction_output(gpio, 1);
>> +             else
>> +                     ret = gpio_direction_output(gpio, 0);
>> +             if (ret) {
>> +                     pr_info("can't make gpio %d an output.\n", gpio);
>> +                     if (gpio_requested)
>> +                             gpio_free(gpio);
>> +                     goto error;
>> +             }
>> +
>> +             tp->gpio_trace.gpio = gpio;
>> +             tp->gpio_trace.action = gpio_actions[i].action;
>> +             argc -= 1; argv += 1;
>> +     }
>> +#endif
>> +
>>       /* parse arguments */
>>       ret = 0;
>>       for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
>> @@ -1145,12 +1260,54 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
>>  }
>>  #endif       /* CONFIG_PERF_EVENTS */
>>
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +static __kprobes void kprobe_gpio_take_action(int gpio, int action)
>> +{
>> +     int val;
>> +     switch (action) {
>> +     case TP_GPIO_CMD_PULSE_HIGH:
>> +             gpio_set_value(gpio, 1);
>> +             /* intentionnaly don't break here */
>> +     case TP_GPIO_CMD_CLEAR:
>> +             gpio_set_value(gpio, 0);
>> +     break;
>> +
>> +     case TP_GPIO_CMD_PULSE_LOW:
>> +             gpio_set_value(gpio, 0);
>> +             /* intentionnaly don't break here */
>> +     case TP_GPIO_CMD_SET:
>> +             gpio_set_value(gpio, 1);
>> +     break;
>> +
>> +     case TP_GPIO_CMD_TOGGLE:
>> +             val = gpio_get_value(gpio);
>> +             gpio_set_value(gpio, val ? 0 : 1);
>> +     break;
>> +
>> +     case TP_GPIO_CMD_PULSE:
>> +             val = gpio_get_value(gpio);
>> +             gpio_set_value(gpio, val ? 0 : 1);
>> +             gpio_set_value(gpio, val);
>> +     break;
>> +     }
>> +}
>> +
>>  /*
>>   * called by perf_trace_init() or __ftrace_set_clr_event() under event_mutex.
>>   *
>>   * kprobe_trace_self_tests_init() does enable_trace_probe/disable_trace_probe
>>   * lockless, but we can't race with this __init function.
>>   */
>> +/* Kprobe gpio handler */
>> +static inline __kprobes void kprobe_gpio_func(struct trace_probe *tp,
>> +                                           struct pt_regs *regs)
>> +{
>> +     if (tp->gpio_trace.action)
>> +             kprobe_gpio_take_action(tp->gpio_trace.gpio,
>> +                                     tp->gpio_trace.action);
>> +}
>> +#endif /* CONFIG_KPROBE_EVENT_GPIO */
>> +
>>  static __kprobes
>>  int kprobe_register(struct ftrace_event_call *event,
>>                   enum trace_reg type, void *data)
>> @@ -1186,6 +1343,9 @@ int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
>>
>>       tp->nhit++;
>>
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +     kprobe_gpio_func(tp, regs);
>> +#endif
>>       if (tp->flags & TP_FLAG_TRACE)
>>               kprobe_trace_func(tp, regs);
>>  #ifdef CONFIG_PERF_EVENTS
>> @@ -1202,6 +1362,9 @@ int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
>>
>>       tp->nhit++;
>>
>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>> +     kprobe_gpio_func(tp, regs);
>> +#endif
>>       if (tp->flags & TP_FLAG_TRACE)
>>               kretprobe_trace_func(tp, ri, regs);
>>  #ifdef CONFIG_PERF_EVENTS
>>
>
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Tracing events with GPIOs

[Tom Zanussi]

Hi Jean-Jacques,

On Tue, 2013-12-17 at 18:22 +0100, Jean-Jacques Hiblot wrote:
> 2013/12/17 Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>:
> > (2013/12/17 9:22), Jean-Jacques Hiblot wrote:
> >> This patch implements a new tracing mechanism based on kprobes and using GPIO.
> >> Debugging with GPIO is very common in the embedded world. At least for those of us
> >> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
> >> This is especially true if the issue results of a hardware/sofware interaction.
> >>
> >> Typical use cases are :
> >> * mixed software/hardware debugging. For example when the software detects a
> >>   situation of interest (typically an error) it toggles a GPIO to trigger the
> >>   oscilloscope acquisition.
> >> * direct latency/duration measurements.
> >
> > Ah, it's interesting to me :)
> >
> > And I think this feature should be built on the event triggers which
> > Tom is working on, instead of kprobes directly, because it allows
> > you to take gpio actions on normal tracepoints, and uprobes too :)
> >
> 
> I'll make a version that uses this framework then. When it's ready
> I'll make some measurements to check the overhead of both solutions.
> 

Yeah, this does look like an interesting application for event triggers
- looking forward to seeing what you come up with...

> Tom,
> is git://git.yoctoproject.org/linux-yocto-contrib
> tzanussi/event-triggers-v11 the right starting point ?
> 

Yep, that's the last iteration - I haven't changed anything since then.

Tom

> Thanks,
> 
> Jean-Jacques
> 
> > Thank you!
> >
> >>
> >> examples:
> >> To trig the oscilloscope whenever a mmc command error:
> >>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
> >>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable
> >>
> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> >> ---
> >>  kernel/trace/Kconfig        |  12 ++++
> >>  kernel/trace/trace_kprobe.c | 167 +++++++++++++++++++++++++++++++++++++++++++-
> >>  2 files changed, 177 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> >> index 015f85a..4228768 100644
> >> --- a/kernel/trace/Kconfig
> >> +++ b/kernel/trace/Kconfig
> >> @@ -420,6 +420,18 @@ config KPROBE_EVENT
> >>         This option is also required by perf-probe subcommand of perf tools.
> >>         If you want to use perf tools, this option is strongly recommended.
> >>
> >> +config KPROBE_EVENT_GPIO
> >> +     depends on KPROBE_EVENT
> >> +     depends on GPIOLIB
> >> +     bool "Enable kprobes-based tracing of events via GPIO"
> >> +     default n
> >> +     help
> >> +             This allows the user to use GPIOs as a tracing tool.The primary
> >> +             purpose of this option is to allow the user to trigger an
> >> +             oscilloscope on software events.
> >> +             The GPIO can be set, cleared, toggled, or pulsed when the event
> >> +             is hit.
> >> +
> >>  config UPROBE_EVENT
> >>       bool "Enable uprobes-based dynamic events"
> >>       depends on ARCH_SUPPORTS_UPROBES
> >> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> >> index dae9541..5d297f5 100644
> >> --- a/kernel/trace/trace_kprobe.c
> >> +++ b/kernel/trace/trace_kprobe.c
> >> @@ -19,6 +19,7 @@
> >>
> >>  #include <linux/module.h>
> >>  #include <linux/uaccess.h>
> >> +#include <linux/gpio.h>
> >>
> >>  #include "trace_probe.h"
> >>
> >> @@ -27,6 +28,33 @@
> >>  /**
> >>   * Kprobe event core functions
> >>   */
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +#define TP_GPIO_UNDEFINED      0
> >> +#define TP_GPIO_CMD_SET                1
> >> +#define TP_GPIO_CMD_CLEAR      2
> >> +#define TP_GPIO_CMD_TOGGLE     3
> >> +#define TP_GPIO_CMD_PULSE      4
> >> +#define TP_GPIO_CMD_PULSE_HIGH 5
> >> +#define TP_GPIO_CMD_PULSE_LOW  6
> >> +
> >> +static const struct {
> >> +     const char *name;
> >> +     int action;
> >> +} gpio_actions[] = {
> >> +     {"gpioset", TP_GPIO_CMD_SET},
> >> +     {"gpioclear", TP_GPIO_CMD_CLEAR},
> >> +     {"gpiotoggle", TP_GPIO_CMD_TOGGLE},
> >> +     {"gpiopulse", TP_GPIO_CMD_PULSE},
> >> +     {"gpiopulsehigh", TP_GPIO_CMD_PULSE_HIGH},
> >> +     {"gpiopulselow", TP_GPIO_CMD_PULSE_LOW},
> >> +};
> >> +
> >> +struct gpio_trace {
> >> +     int gpio;
> >> +     int action;
> >> +};
> >> +#endif
> >> +
> >>  struct trace_probe {
> >>       struct list_head        list;
> >>       struct kretprobe        rp;     /* Use rp.kp for kprobe use */
> >> @@ -38,6 +66,9 @@ struct trace_probe {
> >>       struct list_head        files;
> >>       ssize_t                 size;           /* trace entry size */
> >>       unsigned int            nr_args;
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +     struct gpio_trace               gpio_trace;
> >> +#endif
> >>       struct probe_arg        args[];
> >>  };
> >>
> >> @@ -164,10 +195,24 @@ error:
> >>       return ERR_PTR(ret);
> >>  }
> >>
> >> +static struct trace_probe *find_gpio_probe(int gpio)
> >> +{
> >> +     struct trace_probe *tp;
> >> +
> >> +     list_for_each_entry(tp, &probe_list, list)
> >> +     if ((tp->gpio_trace.action) && (tp->gpio_trace.gpio == gpio))
> >> +             return tp;
> >> +     return NULL;
> >> +}
> >> +
> >>  static void free_trace_probe(struct trace_probe *tp)
> >>  {
> >>       int i;
> >>
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +     if (tp->gpio_trace.action && !find_gpio_probe(tp->gpio_trace.gpio))
> >> +             gpio_free(tp->gpio_trace.gpio);
> >> +#endif
> >>       for (i = 0; i < tp->nr_args; i++)
> >>               traceprobe_free_probe_arg(&tp->args[i]);
> >>
> >> @@ -435,8 +480,14 @@ static int create_trace_probe(int argc, char **argv)
> >>  {
> >>       /*
> >>        * Argument syntax:
> >> -      *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
> >> -      *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
> >> +      *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR
> >> +      *                [GPIOACTION@GPIONUM] [FETCHARGS]
> >> +      *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0]
> >> +      *                   [GPIOACTION@GPIONUM] [FETCHARGS]
> >> +      * Gpio tracing:
> >> +      *   gpio action can be : gpioset, gpioclear, gpiotoggle, gpiopulse,
> >> +      *                        gpiopulsehigh and gpiopulselow
> >> +      *   gpionum is the id of the gpio
> >>        * Fetch args:
> >>        *  $retval     : fetch return value
> >>        *  $stack      : fetch stack address
> >> @@ -459,6 +510,9 @@ static int create_trace_probe(int argc, char **argv)
> >>       unsigned long offset = 0;
> >>       void *addr = NULL;
> >>       char buf[MAX_EVENT_NAME_LEN];
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +     int len;
> >> +#endif
> >>
> >>       /* argc must be >= 1 */
> >>       if (argv[0][0] == 'p')
> >> @@ -541,6 +595,7 @@ static int create_trace_probe(int argc, char **argv)
> >>                       return -EINVAL;
> >>               }
> >>       }
> >> +
> >>       argc -= 2; argv += 2;
> >>
> >>       /* setup a probe */
> >> @@ -562,6 +617,66 @@ static int create_trace_probe(int argc, char **argv)
> >>               return PTR_ERR(tp);
> >>       }
> >>
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +     /* parse the optionnal gpio action argument */
> >> +     for (i = 0; argc && (i < ARRAY_SIZE(gpio_actions)); i++) {
> >> +             len = strlen(gpio_actions[i].name);
> >> +             if (strncmp(argv[0], gpio_actions[i].name, len) == 0)
> >> +                     break;
> >> +     }
> >> +
> >> +     if (argc && (i < ARRAY_SIZE(gpio_actions))) {
> >> +             int gpio;
> >> +             unsigned long ul;
> >> +             int gpio_requested = 0;
> >> +             ret = -EINVAL;
> >> +             if (argv[0][len] != '@') {
> >> +                     pr_info("Syntax error. wrong gpio syntax\n");
> >> +                     goto error;
> >> +             }
> >> +
> >> +             if (kstrtoul(&argv[0][len+1], 0, &ul)) {
> >> +                     pr_info("Failed to parse gpio number.\n");
> >> +                     goto error;
> >> +             }
> >> +             gpio = ul;
> >> +
> >> +             if (!gpio_is_valid(gpio)) {
> >> +                     pr_info("gpio %d is not valid.\n", gpio);
> >> +                     goto error;
> >> +             }
> >> +
> >> +             if (gpio_cansleep(gpio))
> >> +                     pr_info("gpio %d can sleep. This may break your"
> >> +                             "kernel!!!!!!\n", gpio);
> >> +
> >> +             if (!find_gpio_probe(gpio)) {
> >> +                     ret = gpio_request(gpio, event);
> >> +                     if (ret) {
> >> +                             pr_info("can't request gpio %d.\n", gpio);
> >> +                             goto error;
> >> +                     }
> >> +                     gpio_requested = 1;
> >> +             }
> >> +
> >> +             if ((gpio_actions[i].action == TP_GPIO_CMD_CLEAR) ||
> >> +                     (gpio_actions[i].action == TP_GPIO_CMD_PULSE_LOW))
> >> +                     ret = gpio_direction_output(gpio, 1);
> >> +             else
> >> +                     ret = gpio_direction_output(gpio, 0);
> >> +             if (ret) {
> >> +                     pr_info("can't make gpio %d an output.\n", gpio);
> >> +                     if (gpio_requested)
> >> +                             gpio_free(gpio);
> >> +                     goto error;
> >> +             }
> >> +
> >> +             tp->gpio_trace.gpio = gpio;
> >> +             tp->gpio_trace.action = gpio_actions[i].action;
> >> +             argc -= 1; argv += 1;
> >> +     }
> >> +#endif
> >> +
> >>       /* parse arguments */
> >>       ret = 0;
> >>       for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
> >> @@ -1145,12 +1260,54 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
> >>  }
> >>  #endif       /* CONFIG_PERF_EVENTS */
> >>
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +static __kprobes void kprobe_gpio_take_action(int gpio, int action)
> >> +{
> >> +     int val;
> >> +     switch (action) {
> >> +     case TP_GPIO_CMD_PULSE_HIGH:
> >> +             gpio_set_value(gpio, 1);
> >> +             /* intentionnaly don't break here */
> >> +     case TP_GPIO_CMD_CLEAR:
> >> +             gpio_set_value(gpio, 0);
> >> +     break;
> >> +
> >> +     case TP_GPIO_CMD_PULSE_LOW:
> >> +             gpio_set_value(gpio, 0);
> >> +             /* intentionnaly don't break here */
> >> +     case TP_GPIO_CMD_SET:
> >> +             gpio_set_value(gpio, 1);
> >> +     break;
> >> +
> >> +     case TP_GPIO_CMD_TOGGLE:
> >> +             val = gpio_get_value(gpio);
> >> +             gpio_set_value(gpio, val ? 0 : 1);
> >> +     break;
> >> +
> >> +     case TP_GPIO_CMD_PULSE:
> >> +             val = gpio_get_value(gpio);
> >> +             gpio_set_value(gpio, val ? 0 : 1);
> >> +             gpio_set_value(gpio, val);
> >> +     break;
> >> +     }
> >> +}
> >> +
> >>  /*
> >>   * called by perf_trace_init() or __ftrace_set_clr_event() under event_mutex.
> >>   *
> >>   * kprobe_trace_self_tests_init() does enable_trace_probe/disable_trace_probe
> >>   * lockless, but we can't race with this __init function.
> >>   */
> >> +/* Kprobe gpio handler */
> >> +static inline __kprobes void kprobe_gpio_func(struct trace_probe *tp,
> >> +                                           struct pt_regs *regs)
> >> +{
> >> +     if (tp->gpio_trace.action)
> >> +             kprobe_gpio_take_action(tp->gpio_trace.gpio,
> >> +                                     tp->gpio_trace.action);
> >> +}
> >> +#endif /* CONFIG_KPROBE_EVENT_GPIO */
> >> +
> >>  static __kprobes
> >>  int kprobe_register(struct ftrace_event_call *event,
> >>                   enum trace_reg type, void *data)
> >> @@ -1186,6 +1343,9 @@ int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
> >>
> >>       tp->nhit++;
> >>
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +     kprobe_gpio_func(tp, regs);
> >> +#endif
> >>       if (tp->flags & TP_FLAG_TRACE)
> >>               kprobe_trace_func(tp, regs);
> >>  #ifdef CONFIG_PERF_EVENTS
> >> @@ -1202,6 +1362,9 @@ int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
> >>
> >>       tp->nhit++;
> >>
> >> +#ifdef CONFIG_KPROBE_EVENT_GPIO
> >> +     kprobe_gpio_func(tp, regs);
> >> +#endif
> >>       if (tp->flags & TP_FLAG_TRACE)
> >>               kretprobe_trace_func(tp, ri, regs);
> >>  #ifdef CONFIG_PERF_EVENTS
> >>
> >
> >
> > --
> > Masami HIRAMATSU
> > IT Management Research Dept. Linux Technology Center
> > Hitachi, Ltd., Yokohama Research Laboratory
> > E-mail: masami.hiramatsu.pt@hitachi.com
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Tracing events with GPIOs

[Steven Rostedt]

On Tue, 17 Dec 2013 12:29:30 -0600
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> > Tom,
> > is git://git.yoctoproject.org/linux-yocto-contrib
> > tzanussi/event-triggers-v11 the right starting point ?
> > 
> 
> Yep, that's the last iteration - I haven't changed anything since then.
> 

Is this the same as the last patch series you sent out? I started
testing them again last week, but they failed some of my tests.
Unfortunately, I didn't get a chance to look into why. I'm hoping to
get that series out to my for-next branch by end of the week, as I'll
be taking two weeks off starting next week.

Year end crap is piling up, preventing me from getting real work
done ;-)

-- Steve

Re: [PATCH] Tracing events with GPIOs

[Tom Zanussi]

On Tue, 2013-12-17 at 14:05 -0500, Steven Rostedt wrote:
> On Tue, 17 Dec 2013 12:29:30 -0600
> Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> 
> > > Tom,
> > > is git://git.yoctoproject.org/linux-yocto-contrib
> > > tzanussi/event-triggers-v11 the right starting point ?
> > > 
> > 
> > Yep, that's the last iteration - I haven't changed anything since then.
> > 
> 
> Is this the same as the last patch series you sent out? I started
> testing them again last week, but they failed some of my tests.

Yeah, that's the same patchset.  I tested it here and didn't see any
problems but I guess your tests must be different/more rigorous - it
would be good to know what failed.  I could also run your test suite
here if that would help.

> Unfortunately, I didn't get a chance to look into why. I'm hoping to
> get that series out to my for-next branch by end of the week, as I'll
> be taking two weeks off starting next week.
> 
> Year end crap is piling up, preventing me from getting real work
> done ;-)

I know what you mean, same here.  ;-)

> 
> -- Steve

Re: [PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

2013/12/17 Jean-Jacques Hiblot <jjhiblot@traphandler.com>:
> 2013/12/17 Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>:
>> (2013/12/17 9:22), Jean-Jacques Hiblot wrote:
>>> This patch implements a new tracing mechanism based on kprobes and using GPIO.
>>> Debugging with GPIO is very common in the embedded world. At least for those of us
>>> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
>>> This is especially true if the issue results of a hardware/sofware interaction.
>>>
>>> Typical use cases are :
>>> * mixed software/hardware debugging. For example when the software detects a
>>>   situation of interest (typically an error) it toggles a GPIO to trigger the
>>>   oscilloscope acquisition.
>>> * direct latency/duration measurements.
>>
>> Ah, it's interesting to me :)
>>
>> And I think this feature should be built on the event triggers which
>> Tom is working on, instead of kprobes directly, because it allows
>> you to take gpio actions on normal tracepoints, and uprobes too :)
>>
>
> I'll make a version that uses this framework then. When it's ready
> I'll make some measurements to check the overhead of both solutions.


Here are the results. The test is run on an old ARM9 platform
(at91sam9261ek) running at 200MHz
The test consists in setting the GPIO when entering handle_IRQ() and
clearing the GPIO when entering handle_fasteoi_irq(), and measuring
the width of the pulse with an oscilloscope.

1) manual method (modification of the code to insert gpio_set_value()
at the top of each probed function). This is our baseline
min : 2.76 us
mean : 3.22 us
max : 6.80 us

2) pure kprobe (patch v1)
echo "" > /sys/kernel/debug/tracing/kprobe_events
echo "p:b handle_IRQ gpioset@13" > /sys/kernel/debug/tracing/kprobe_events
echo "p:a handle_fasteoi_irq gpioclear@13" >>
/sys/kernel/debug/tracing/kprobe_events
echo 1 > /sys/kernel/debug/tracing/events/kprobes/a/enable
echo 1 > /sys/kernel/debug/tracing/events/kprobes/b/enable

min : 18.2 us
mean 19.2 us
max : 34.0 us

3) event trigger (patch v2)
echo "" > /sys/kernel/debug/tracing/kprobe_events
echo "p:b handle_IRQ" > /sys/kernel/debug/tracing/kprobe_events
echo "p:a handle_fasteoi_irq" >> /sys/kernel/debug/tracing/kprobe_events
echo 'gpio:clear:13' > /sys/kernel/debug/tracing/events/kprobes/a/trigger
echo 'gpio:set:13' > /sys/kernel/debug/tracing/events/kprobes/b/trigger

min : 7.36 us
mean : 8.32 us
max : 14.8 us

>From those numbers we deduce the cost of an event triggered gpio probe
: (8.32 - 3.22)/2 = 2.55 us (max 4 us).

>
> Tom,
> is git://git.yoctoproject.org/linux-yocto-contrib
> tzanussi/event-triggers-v11 the right starting point ?
>
> Thanks,
>
> Jean-Jacques
>
>> Thank you!
>>
>>>
>>> examples:
>>> To trig the oscilloscope whenever a mmc command error:
>>>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>>>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable
>>>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> ---
>>>  kernel/trace/Kconfig        |  12 ++++
>>>  kernel/trace/trace_kprobe.c | 167 +++++++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 177 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
>>> index 015f85a..4228768 100644
>>> --- a/kernel/trace/Kconfig
>>> +++ b/kernel/trace/Kconfig
>>> @@ -420,6 +420,18 @@ config KPROBE_EVENT
>>>         This option is also required by perf-probe subcommand of perf tools.
>>>         If you want to use perf tools, this option is strongly recommended.
>>>
>>> +config KPROBE_EVENT_GPIO
>>> +     depends on KPROBE_EVENT
>>> +     depends on GPIOLIB
>>> +     bool "Enable kprobes-based tracing of events via GPIO"
>>> +     default n
>>> +     help
>>> +             This allows the user to use GPIOs as a tracing tool.The primary
>>> +             purpose of this option is to allow the user to trigger an
>>> +             oscilloscope on software events.
>>> +             The GPIO can be set, cleared, toggled, or pulsed when the event
>>> +             is hit.
>>> +
>>>  config UPROBE_EVENT
>>>       bool "Enable uprobes-based dynamic events"
>>>       depends on ARCH_SUPPORTS_UPROBES
>>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
>>> index dae9541..5d297f5 100644
>>> --- a/kernel/trace/trace_kprobe.c
>>> +++ b/kernel/trace/trace_kprobe.c
>>> @@ -19,6 +19,7 @@
>>>
>>>  #include <linux/module.h>
>>>  #include <linux/uaccess.h>
>>> +#include <linux/gpio.h>
>>>
>>>  #include "trace_probe.h"
>>>
>>> @@ -27,6 +28,33 @@
>>>  /**
>>>   * Kprobe event core functions
>>>   */
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +#define TP_GPIO_UNDEFINED      0
>>> +#define TP_GPIO_CMD_SET                1
>>> +#define TP_GPIO_CMD_CLEAR      2
>>> +#define TP_GPIO_CMD_TOGGLE     3
>>> +#define TP_GPIO_CMD_PULSE      4
>>> +#define TP_GPIO_CMD_PULSE_HIGH 5
>>> +#define TP_GPIO_CMD_PULSE_LOW  6
>>> +
>>> +static const struct {
>>> +     const char *name;
>>> +     int action;
>>> +} gpio_actions[] = {
>>> +     {"gpioset", TP_GPIO_CMD_SET},
>>> +     {"gpioclear", TP_GPIO_CMD_CLEAR},
>>> +     {"gpiotoggle", TP_GPIO_CMD_TOGGLE},
>>> +     {"gpiopulse", TP_GPIO_CMD_PULSE},
>>> +     {"gpiopulsehigh", TP_GPIO_CMD_PULSE_HIGH},
>>> +     {"gpiopulselow", TP_GPIO_CMD_PULSE_LOW},
>>> +};
>>> +
>>> +struct gpio_trace {
>>> +     int gpio;
>>> +     int action;
>>> +};
>>> +#endif
>>> +
>>>  struct trace_probe {
>>>       struct list_head        list;
>>>       struct kretprobe        rp;     /* Use rp.kp for kprobe use */
>>> @@ -38,6 +66,9 @@ struct trace_probe {
>>>       struct list_head        files;
>>>       ssize_t                 size;           /* trace entry size */
>>>       unsigned int            nr_args;
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +     struct gpio_trace               gpio_trace;
>>> +#endif
>>>       struct probe_arg        args[];
>>>  };
>>>
>>> @@ -164,10 +195,24 @@ error:
>>>       return ERR_PTR(ret);
>>>  }
>>>
>>> +static struct trace_probe *find_gpio_probe(int gpio)
>>> +{
>>> +     struct trace_probe *tp;
>>> +
>>> +     list_for_each_entry(tp, &probe_list, list)
>>> +     if ((tp->gpio_trace.action) && (tp->gpio_trace.gpio == gpio))
>>> +             return tp;
>>> +     return NULL;
>>> +}
>>> +
>>>  static void free_trace_probe(struct trace_probe *tp)
>>>  {
>>>       int i;
>>>
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +     if (tp->gpio_trace.action && !find_gpio_probe(tp->gpio_trace.gpio))
>>> +             gpio_free(tp->gpio_trace.gpio);
>>> +#endif
>>>       for (i = 0; i < tp->nr_args; i++)
>>>               traceprobe_free_probe_arg(&tp->args[i]);
>>>
>>> @@ -435,8 +480,14 @@ static int create_trace_probe(int argc, char **argv)
>>>  {
>>>       /*
>>>        * Argument syntax:
>>> -      *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
>>> -      *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
>>> +      *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR
>>> +      *                [GPIOACTION@GPIONUM] [FETCHARGS]
>>> +      *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0]
>>> +      *                   [GPIOACTION@GPIONUM] [FETCHARGS]
>>> +      * Gpio tracing:
>>> +      *   gpio action can be : gpioset, gpioclear, gpiotoggle, gpiopulse,
>>> +      *                        gpiopulsehigh and gpiopulselow
>>> +      *   gpionum is the id of the gpio
>>>        * Fetch args:
>>>        *  $retval     : fetch return value
>>>        *  $stack      : fetch stack address
>>> @@ -459,6 +510,9 @@ static int create_trace_probe(int argc, char **argv)
>>>       unsigned long offset = 0;
>>>       void *addr = NULL;
>>>       char buf[MAX_EVENT_NAME_LEN];
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +     int len;
>>> +#endif
>>>
>>>       /* argc must be >= 1 */
>>>       if (argv[0][0] == 'p')
>>> @@ -541,6 +595,7 @@ static int create_trace_probe(int argc, char **argv)
>>>                       return -EINVAL;
>>>               }
>>>       }
>>> +
>>>       argc -= 2; argv += 2;
>>>
>>>       /* setup a probe */
>>> @@ -562,6 +617,66 @@ static int create_trace_probe(int argc, char **argv)
>>>               return PTR_ERR(tp);
>>>       }
>>>
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +     /* parse the optionnal gpio action argument */
>>> +     for (i = 0; argc && (i < ARRAY_SIZE(gpio_actions)); i++) {
>>> +             len = strlen(gpio_actions[i].name);
>>> +             if (strncmp(argv[0], gpio_actions[i].name, len) == 0)
>>> +                     break;
>>> +     }
>>> +
>>> +     if (argc && (i < ARRAY_SIZE(gpio_actions))) {
>>> +             int gpio;
>>> +             unsigned long ul;
>>> +             int gpio_requested = 0;
>>> +             ret = -EINVAL;
>>> +             if (argv[0][len] != '@') {
>>> +                     pr_info("Syntax error. wrong gpio syntax\n");
>>> +                     goto error;
>>> +             }
>>> +
>>> +             if (kstrtoul(&argv[0][len+1], 0, &ul)) {
>>> +                     pr_info("Failed to parse gpio number.\n");
>>> +                     goto error;
>>> +             }
>>> +             gpio = ul;
>>> +
>>> +             if (!gpio_is_valid(gpio)) {
>>> +                     pr_info("gpio %d is not valid.\n", gpio);
>>> +                     goto error;
>>> +             }
>>> +
>>> +             if (gpio_cansleep(gpio))
>>> +                     pr_info("gpio %d can sleep. This may break your"
>>> +                             "kernel!!!!!!\n", gpio);
>>> +
>>> +             if (!find_gpio_probe(gpio)) {
>>> +                     ret = gpio_request(gpio, event);
>>> +                     if (ret) {
>>> +                             pr_info("can't request gpio %d.\n", gpio);
>>> +                             goto error;
>>> +                     }
>>> +                     gpio_requested = 1;
>>> +             }
>>> +
>>> +             if ((gpio_actions[i].action == TP_GPIO_CMD_CLEAR) ||
>>> +                     (gpio_actions[i].action == TP_GPIO_CMD_PULSE_LOW))
>>> +                     ret = gpio_direction_output(gpio, 1);
>>> +             else
>>> +                     ret = gpio_direction_output(gpio, 0);
>>> +             if (ret) {
>>> +                     pr_info("can't make gpio %d an output.\n", gpio);
>>> +                     if (gpio_requested)
>>> +                             gpio_free(gpio);
>>> +                     goto error;
>>> +             }
>>> +
>>> +             tp->gpio_trace.gpio = gpio;
>>> +             tp->gpio_trace.action = gpio_actions[i].action;
>>> +             argc -= 1; argv += 1;
>>> +     }
>>> +#endif
>>> +
>>>       /* parse arguments */
>>>       ret = 0;
>>>       for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
>>> @@ -1145,12 +1260,54 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
>>>  }
>>>  #endif       /* CONFIG_PERF_EVENTS */
>>>
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +static __kprobes void kprobe_gpio_take_action(int gpio, int action)
>>> +{
>>> +     int val;
>>> +     switch (action) {
>>> +     case TP_GPIO_CMD_PULSE_HIGH:
>>> +             gpio_set_value(gpio, 1);
>>> +             /* intentionnaly don't break here */
>>> +     case TP_GPIO_CMD_CLEAR:
>>> +             gpio_set_value(gpio, 0);
>>> +     break;
>>> +
>>> +     case TP_GPIO_CMD_PULSE_LOW:
>>> +             gpio_set_value(gpio, 0);
>>> +             /* intentionnaly don't break here */
>>> +     case TP_GPIO_CMD_SET:
>>> +             gpio_set_value(gpio, 1);
>>> +     break;
>>> +
>>> +     case TP_GPIO_CMD_TOGGLE:
>>> +             val = gpio_get_value(gpio);
>>> +             gpio_set_value(gpio, val ? 0 : 1);
>>> +     break;
>>> +
>>> +     case TP_GPIO_CMD_PULSE:
>>> +             val = gpio_get_value(gpio);
>>> +             gpio_set_value(gpio, val ? 0 : 1);
>>> +             gpio_set_value(gpio, val);
>>> +     break;
>>> +     }
>>> +}
>>> +
>>>  /*
>>>   * called by perf_trace_init() or __ftrace_set_clr_event() under event_mutex.
>>>   *
>>>   * kprobe_trace_self_tests_init() does enable_trace_probe/disable_trace_probe
>>>   * lockless, but we can't race with this __init function.
>>>   */
>>> +/* Kprobe gpio handler */
>>> +static inline __kprobes void kprobe_gpio_func(struct trace_probe *tp,
>>> +                                           struct pt_regs *regs)
>>> +{
>>> +     if (tp->gpio_trace.action)
>>> +             kprobe_gpio_take_action(tp->gpio_trace.gpio,
>>> +                                     tp->gpio_trace.action);
>>> +}
>>> +#endif /* CONFIG_KPROBE_EVENT_GPIO */
>>> +
>>>  static __kprobes
>>>  int kprobe_register(struct ftrace_event_call *event,
>>>                   enum trace_reg type, void *data)
>>> @@ -1186,6 +1343,9 @@ int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
>>>
>>>       tp->nhit++;
>>>
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +     kprobe_gpio_func(tp, regs);
>>> +#endif
>>>       if (tp->flags & TP_FLAG_TRACE)
>>>               kprobe_trace_func(tp, regs);
>>>  #ifdef CONFIG_PERF_EVENTS
>>> @@ -1202,6 +1362,9 @@ int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
>>>
>>>       tp->nhit++;
>>>
>>> +#ifdef CONFIG_KPROBE_EVENT_GPIO
>>> +     kprobe_gpio_func(tp, regs);
>>> +#endif
>>>       if (tp->flags & TP_FLAG_TRACE)
>>>               kretprobe_trace_func(tp, ri, regs);
>>>  #ifdef CONFIG_PERF_EVENTS
>>>
>>
>>
>> --
>> Masami HIRAMATSU
>> IT Management Research Dept. Linux Technology Center
>> Hitachi, Ltd., Yokohama Research Laboratory
>> E-mail: masami.hiramatsu.pt@hitachi.com
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Tracing events with GPIOs

[Alexandre Courbot]

On Tue, Dec 17, 2013 at 9:22 AM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
>
> This patch implements a new tracing mechanism based on kprobes and using GPIO.
> Debugging with GPIO is very common in the embedded world. At least for those of us
> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
> This is especially true if the issue results of a hardware/sofware interaction.
>
> Typical use cases are :
> * mixed software/hardware debugging. For example when the software detects a
>   situation of interest (typically an error) it toggles a GPIO to trigger the
>   oscilloscope acquisition.
> * direct latency/duration measurements.
>
> examples:
> To trig the oscilloscope whenever a mmc command error:
>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable

I do like this idea, however I wonder if you could try and make it use
the new gpio descriptor API (see Documentation/gpio/) instead of the
GPIO integers we are trying to deprecate (well ok, we just *started*
claiming they are deprecated).

This would probably make things a little bit more complicated on your
side, due to the fact the gpiod API is new and probably does not cover
all your needs. But it would also make your approach safer and more
future proof, on top of helping us refine gpiod for various use cases.

The problems I can see so far:

- Using gpiod, GPIOs are not specified as integers, but are typically
mapped to a given (device, function) pair (device can be NULL) using
device tree/platform data/ACPI and obtained by the corresponding
device driver through gpiod_get(). You would need to find a different
way to specify GPIOs, maybe using the gpio_chip's label and the GPIO
hardware number.

- Even if you do so, there is currently no way to arbitrarily obtain a
GPIO that has not been explicitly mapped to a (device, function), and
IIUC you need to specify the tracing GPIO freely from user-space. This
hints that we will need to add a function that is sensibly the same as
gpio_request_one() to the gpiod API, but I wonder if that does not
defeats the purpose somehow.

So using gpiod we would have the dual problem of how to represent the
GPIO you need from user-space, and how you can safely obtain it. It
would be interesting to hear what Linus thinks about it, and if he has
better ideas about how we could solve these issues (as he usually has
;) ).

(note that it is *not* a hard requirement to use gpiod over the legacy
integer API, but considering this is the direction we are taking, it
would be nice to consider it and see how we could solve the issues
mentioned above)

Thanks,
Alex.

Re: [PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

2013/12/19 Alexandre Courbot <gnurou@gmail.com>:
> On Tue, Dec 17, 2013 at 9:22 AM, Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>>
>> This patch implements a new tracing mechanism based on kprobes and using GPIO.
>> Debugging with GPIO is very common in the embedded world. At least for those of us
>> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
>> This is especially true if the issue results of a hardware/sofware interaction.
>>
>> Typical use cases are :
>> * mixed software/hardware debugging. For example when the software detects a
>>   situation of interest (typically an error) it toggles a GPIO to trigger the
>>   oscilloscope acquisition.
>> * direct latency/duration measurements.
>>
>> examples:
>> To trig the oscilloscope whenever a mmc command error:
>>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable
>
> I do like this idea, however I wonder if you could try and make it use
> the new gpio descriptor API (see Documentation/gpio/) instead of the
> GPIO integers we are trying to deprecate (well ok, we just *started*
> claiming they are deprecated).
>
I'll make the gpio event trigger work with the old API first. Then
I'll adapt it to use the gpio descriptor interface.

> This would probably make things a little bit more complicated on your
> side, due to the fact the gpiod API is new and probably does not cover
> all your needs. But it would also make your approach safer and more
> future proof, on top of helping us refine gpiod for various use cases.
>
> The problems I can see so far:
>
> - Using gpiod, GPIOs are not specified as integers, but are typically
> mapped to a given (device, function) pair (device can be NULL) using
> device tree/platform data/ACPI and obtained by the corresponding
> device driver through gpiod_get(). You would need to find a different
> way to specify GPIOs, maybe using the gpio_chip's label and the GPIO
> hardware number.
>
> - Even if you do so, there is currently no way to arbitrarily obtain a
> GPIO that has not been explicitly mapped to a (device, function), and
> IIUC you need to specify the tracing GPIO freely from user-space. This
> hints that we will need to add a function that is sensibly the same as
> gpio_request_one() to the gpiod API, but I wonder if that does not
> defeats the purpose somehow.

This is something I was wondering about for another reason. In many
cases the GPIOs that are physically available for probing will be
limited to the GPIOs already assigned a function (backlight control
for example), others are usually not routed except in eval boards or
early prototypes. And consequently those GPIOs will be requested by a
driver long before a probe is set.
It would be nice not to have to remove the driver to be able to use
this GPIO  as a probe. Maybe a gpiod_steal() interface and a flag
indicating that the GPIO can be safely stolen?

>
> So using gpiod we would have the dual problem of how to represent the
> GPIO you need from user-space, and how you can safely obtain it. It
> would be interesting to hear what Linus thinks about it, and if he has
> better ideas about how we could solve these issues (as he usually has
> ;) ).
>
> (note that it is *not* a hard requirement to use gpiod over the legacy
> integer API, but considering this is the direction we are taking, it
> would be nice to consider it and see how we could solve the issues
> mentioned above)
>
> Thanks,
> Alex.

Re: [PATCH] Tracing events with GPIOs

[Alexandre Courbot]

On Thu, Dec 19, 2013 at 8:38 PM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
> 2013/12/19 Alexandre Courbot <gnurou@gmail.com>:
>> The problems I can see so far:
>>
>> - Using gpiod, GPIOs are not specified as integers, but are typically
>> mapped to a given (device, function) pair (device can be NULL) using
>> device tree/platform data/ACPI and obtained by the corresponding
>> device driver through gpiod_get(). You would need to find a different
>> way to specify GPIOs, maybe using the gpio_chip's label and the GPIO
>> hardware number.
>>
>> - Even if you do so, there is currently no way to arbitrarily obtain a
>> GPIO that has not been explicitly mapped to a (device, function), and
>> IIUC you need to specify the tracing GPIO freely from user-space. This
>> hints that we will need to add a function that is sensibly the same as
>> gpio_request_one() to the gpiod API, but I wonder if that does not
>> defeats the purpose somehow.
>
> This is something I was wondering about for another reason. In many
> cases the GPIOs that are physically available for probing will be
> limited to the GPIOs already assigned a function (backlight control
> for example), others are usually not routed except in eval boards or
> early prototypes. And consequently those GPIOs will be requested by a
> driver long before a probe is set.
> It would be nice not to have to remove the driver to be able to use
> this GPIO  as a probe. Maybe a gpiod_steal() interface and a flag
> indicating that the GPIO can be safely stolen?

Mmm an explicit way to hijack a GPIO does not sound very safe. Do you
have concrete cases where you need to do so? I guess most boards you
may want to use this patch with would have at least some spare GPIOs
with pins somewhere on the board for this kind of purpose.

Re: [PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

2013/12/20 Alexandre Courbot <gnurou@gmail.com>:
> On Thu, Dec 19, 2013 at 8:38 PM, Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>> 2013/12/19 Alexandre Courbot <gnurou@gmail.com>:
>>> The problems I can see so far:
>>>
>>> - Using gpiod, GPIOs are not specified as integers, but are typically
>>> mapped to a given (device, function) pair (device can be NULL) using
>>> device tree/platform data/ACPI and obtained by the corresponding
>>> device driver through gpiod_get(). You would need to find a different
>>> way to specify GPIOs, maybe using the gpio_chip's label and the GPIO
>>> hardware number.
>>>
>>> - Even if you do so, there is currently no way to arbitrarily obtain a
>>> GPIO that has not been explicitly mapped to a (device, function), and
>>> IIUC you need to specify the tracing GPIO freely from user-space. This
>>> hints that we will need to add a function that is sensibly the same as
>>> gpio_request_one() to the gpiod API, but I wonder if that does not
>>> defeats the purpose somehow.
>>
>> This is something I was wondering about for another reason. In many
>> cases the GPIOs that are physically available for probing will be
>> limited to the GPIOs already assigned a function (backlight control
>> for example), others are usually not routed except in eval boards or
>> early prototypes. And consequently those GPIOs will be requested by a
>> driver long before a probe is set.
>> It would be nice not to have to remove the driver to be able to use
>> this GPIO  as a probe. Maybe a gpiod_steal() interface and a flag
>> indicating that the GPIO can be safely stolen?
>
> Mmm an explicit way to hijack a GPIO does not sound very safe. Do you
> have concrete cases where you need to do so? I guess most boards you
> may want to use this patch with would have at least some spare GPIOs
> with pins somewhere on the board for this kind of purpose.
It's not always true. There are quite a few platforms where GPIOs is a
scarce resource (ppc for example). For example, the board I'm working
on at the moment is built around a APM powerpc which has only 16
GPIOs. Of those 16 GPIOS, some are not routed and most of the others
are hidden by the shielding so that I can probe only those that go to
external connectors.
IMHO it's probably the case for most of the boards that go into a
final product where EMI and space constraints are tight.
But I agree that's not safe. I thought that maybe a flag indicating
when it is safe would help (on my board that would be : ok to use the
GPIO that turns on or off the backlight, not ok to use the GPIO that
controls the power supply)

Re: [PATCH] Tracing events with GPIOs

[Alexandre Courbot]

On Fri, Dec 20, 2013 at 5:40 PM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
> 2013/12/20 Alexandre Courbot <gnurou@gmail.com>:
>> On Thu, Dec 19, 2013 at 8:38 PM, Jean-Jacques Hiblot
>> <jjhiblot@traphandler.com> wrote:
>>> 2013/12/19 Alexandre Courbot <gnurou@gmail.com>:
>>>> The problems I can see so far:
>>>>
>>>> - Using gpiod, GPIOs are not specified as integers, but are typically
>>>> mapped to a given (device, function) pair (device can be NULL) using
>>>> device tree/platform data/ACPI and obtained by the corresponding
>>>> device driver through gpiod_get(). You would need to find a different
>>>> way to specify GPIOs, maybe using the gpio_chip's label and the GPIO
>>>> hardware number.
>>>>
>>>> - Even if you do so, there is currently no way to arbitrarily obtain a
>>>> GPIO that has not been explicitly mapped to a (device, function), and
>>>> IIUC you need to specify the tracing GPIO freely from user-space. This
>>>> hints that we will need to add a function that is sensibly the same as
>>>> gpio_request_one() to the gpiod API, but I wonder if that does not
>>>> defeats the purpose somehow.
>>>
>>> This is something I was wondering about for another reason. In many
>>> cases the GPIOs that are physically available for probing will be
>>> limited to the GPIOs already assigned a function (backlight control
>>> for example), others are usually not routed except in eval boards or
>>> early prototypes. And consequently those GPIOs will be requested by a
>>> driver long before a probe is set.
>>> It would be nice not to have to remove the driver to be able to use
>>> this GPIO  as a probe. Maybe a gpiod_steal() interface and a flag
>>> indicating that the GPIO can be safely stolen?
>>
>> Mmm an explicit way to hijack a GPIO does not sound very safe. Do you
>> have concrete cases where you need to do so? I guess most boards you
>> may want to use this patch with would have at least some spare GPIOs
>> with pins somewhere on the board for this kind of purpose.
> It's not always true. There are quite a few platforms where GPIOs is a
> scarce resource (ppc for example). For example, the board I'm working
> on at the moment is built around a APM powerpc which has only 16
> GPIOs. Of those 16 GPIOS, some are not routed and most of the others
> are hidden by the shielding so that I can probe only those that go to
> external connectors.
> IMHO it's probably the case for most of the boards that go into a
> final product where EMI and space constraints are tight.
> But I agree that's not safe. I thought that maybe a flag indicating
> when it is safe would help (on my board that would be : ok to use the
> GPIO that turns on or off the backlight, not ok to use the GPIO that
> controls the power supply)

Rather than a flag per GPIO or board (my definition of "safe" is
rather strict ; I'd consider being able to mess with the backlight as
unsafe), I think I'd be more comfortable with having this as a kernel
config option, defaulting to 'n', and clearly stating that this can
potentially allow any GPIO to be hijacked (which is, apart for the
last point, what you already did). That way people can make an
informed decision about whether to enable it or not (disabled for
distro kernels, enabled for the occasional hacker who wants to use
this feature).

But before doing this, I'd like to make sure we explore every
possibility to make this safer by design.

Re: [PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

2014/1/3 Alexandre Courbot <gnurou@gmail.com>:
> On Fri, Dec 20, 2013 at 5:40 PM, Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>> 2013/12/20 Alexandre Courbot <gnurou@gmail.com>:
>>> On Thu, Dec 19, 2013 at 8:38 PM, Jean-Jacques Hiblot
>>> <jjhiblot@traphandler.com> wrote:
>>>> 2013/12/19 Alexandre Courbot <gnurou@gmail.com>:
>>>>> The problems I can see so far:
>>>>>
>>>>> - Using gpiod, GPIOs are not specified as integers, but are typically
>>>>> mapped to a given (device, function) pair (device can be NULL) using
>>>>> device tree/platform data/ACPI and obtained by the corresponding
>>>>> device driver through gpiod_get(). You would need to find a different
>>>>> way to specify GPIOs, maybe using the gpio_chip's label and the GPIO
>>>>> hardware number.
>>>>>
>>>>> - Even if you do so, there is currently no way to arbitrarily obtain a
>>>>> GPIO that has not been explicitly mapped to a (device, function), and
>>>>> IIUC you need to specify the tracing GPIO freely from user-space. This
>>>>> hints that we will need to add a function that is sensibly the same as
>>>>> gpio_request_one() to the gpiod API, but I wonder if that does not
>>>>> defeats the purpose somehow.
>>>>
>>>> This is something I was wondering about for another reason. In many
>>>> cases the GPIOs that are physically available for probing will be
>>>> limited to the GPIOs already assigned a function (backlight control
>>>> for example), others are usually not routed except in eval boards or
>>>> early prototypes. And consequently those GPIOs will be requested by a
>>>> driver long before a probe is set.
>>>> It would be nice not to have to remove the driver to be able to use
>>>> this GPIO  as a probe. Maybe a gpiod_steal() interface and a flag
>>>> indicating that the GPIO can be safely stolen?
>>>
>>> Mmm an explicit way to hijack a GPIO does not sound very safe. Do you
>>> have concrete cases where you need to do so? I guess most boards you
>>> may want to use this patch with would have at least some spare GPIOs
>>> with pins somewhere on the board for this kind of purpose.
>> It's not always true. There are quite a few platforms where GPIOs is a
>> scarce resource (ppc for example). For example, the board I'm working
>> on at the moment is built around a APM powerpc which has only 16
>> GPIOs. Of those 16 GPIOS, some are not routed and most of the others
>> are hidden by the shielding so that I can probe only those that go to
>> external connectors.
>> IMHO it's probably the case for most of the boards that go into a
>> final product where EMI and space constraints are tight.
>> But I agree that's not safe. I thought that maybe a flag indicating
>> when it is safe would help (on my board that would be : ok to use the
>> GPIO that turns on or off the backlight, not ok to use the GPIO that
>> controls the power supply)
>
> Rather than a flag per GPIO or board (my definition of "safe" is
> rather strict ; I'd consider being able to mess with the backlight as
> unsafe), I think I'd be more comfortable with having this as a kernel
> config option, defaulting to 'n', and clearly stating that this can
> potentially allow any GPIO to be hijacked (which is, apart for the
> last point, what you already did). That way people can make an
> informed decision about whether to enable it or not (disabled for
> distro kernels, enabled for the occasional hacker who wants to use
> this feature).
>
> But before doing this, I'd like to make sure we explore every
> possibility to make this safer by design.
I agree and to be honest I'm not comfortable with the hijacker's way.
Maybe I should just stick to the proper way of requesting a GPIO. When
this feature is used,  it'll not be in the field after production but
during the development when building a new kernel is not really an
issue.

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

Re: [PATCH] Tracing events with GPIOs

[Linus Walleij]

On Thu, Dec 19, 2013 at 7:52 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Tue, Dec 17, 2013 at 9:22 AM, Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>>
>> This patch implements a new tracing mechanism based on kprobes and using GPIO.
>> Debugging with GPIO is very common in the embedded world. At least for those of us
>> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
>> This is especially true if the issue results of a hardware/sofware interaction.
>>
>> Typical use cases are :
>> * mixed software/hardware debugging. For example when the software detects a
>>   situation of interest (typically an error) it toggles a GPIO to trigger the
>>   oscilloscope acquisition.
>> * direct latency/duration measurements.
>>
>> examples:
>> To trig the oscilloscope whenever a mmc command error:
>>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable

I've never seen the whole patch.

Can you please repost this and include linux-gpio@vger.kernel.org and also
me on the To: line?

Yours,
Linus Walleij

Re: [PATCH] Tracing events with GPIOs

[Jean-Jacques Hiblot]

2014/1/2 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Dec 19, 2013 at 7:52 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Tue, Dec 17, 2013 at 9:22 AM, Jean-Jacques Hiblot
>> <jjhiblot@traphandler.com> wrote:
>>>
>>> This patch implements a new tracing mechanism based on kprobes and using GPIO.
>>> Debugging with GPIO is very common in the embedded world. At least for those of us
>>> fortunate enough to have an oscilloscope or a logic analyzer on their bench...
>>> This is especially true if the issue results of a hardware/sofware interaction.
>>>
>>> Typical use cases are :
>>> * mixed software/hardware debugging. For example when the software detects a
>>>   situation of interest (typically an error) it toggles a GPIO to trigger the
>>>   oscilloscope acquisition.
>>> * direct latency/duration measurements.
>>>
>>> examples:
>>> To trig the oscilloscope whenever a mmc command error:
>>>   echo "p:my_mmc_blk_error mmc_blk_cmd_error gpiopulse@13" > /sys/kernel/debug/tracing/kprobe_events
>>>   echo 1 > /sys/kernel/debug/tracing/events/kprobes/my_mmc_blk_error/enable
>
> I've never seen the whole patch.
>
> Can you please repost this and include linux-gpio@vger.kernel.org and also
> me on the To: line?

Hello Linus,

I'll repost the patch.
It has evolved a bit since you've been included in the cc list. It's
now based upon the 'event trigger' and more important for you, it
doesn't request nor configure the GPIO before using it. The reason is
that how to do this is still being discussed with Alexandre.

Jean-Jacques

>
> Yours,
> Linus Walleij