line6linux-devel — mailing list for developers of Line6 Linux software

[Line6linux-devel] [PATCH 02/46] staging: line6: drop clip sysfs attr

[Stefan H. <ste...@gm...>]

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/pod.c | 23 -----------------------
 drivers/staging/line6/pod.h |  5 -----
 2 files changed, 28 deletions(-)

diff --git a/drivers/staging/line6/pod.c b/drivers/staging/line6/pod.c
index e3e2247..4abe2de 100644
--- a/drivers/staging/line6/pod.c
+++ b/drivers/staging/line6/pod.c
@@ -26,7 +26,6 @@
 /* *INDENT-OFF* */
 
 enum {
-	POD_SYSEX_CLIP      = 0x0f,
 	POD_SYSEX_SAVE      = 0x24,
 	POD_SYSEX_SYSTEM    = 0x56,
 	POD_SYSEX_SYSTEMREQ = 0x57,
@@ -311,12 +310,6 @@ void line6_pod_process_message(struct usb_line6_pod *pod)
 				pod_save_button_pressed(pod, buf[6], buf[7]);
 				break;
 
-			case POD_SYSEX_CLIP:
-				dev_dbg(pod->line6.ifcdev, "audio clipped\n");
-				pod->clipping.value = 1;
-				wake_up(&pod->clipping.wait);
-				break;
-
 			case POD_SYSEX_STORE:
 				dev_dbg(pod->line6.ifcdev,
 					"message %02X not yet implemented\n",
@@ -879,18 +872,6 @@ static ssize_t pod_get_device_id(struct device *dev,
 }
 
 /*
-	"read" request on "clip" special file.
-*/
-static ssize_t pod_wait_for_clip(struct device *dev,
-				 struct device_attribute *attr, char *buf)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-	return wait_event_interruptible(pod->clipping.wait,
-					pod->clipping.value != 0);
-}
-
-/*
 	POD startup procedure.
 	This is a sequence of functions with special requirements (e.g., must
 	not run immediately after initialization, must not run in interrupt
@@ -991,7 +972,6 @@ POD_GET_SYSTEM_PARAM(tuner_pitch, 1);
 #undef GET_SYSTEM_PARAM
 
 /* POD special files: */
-static DEVICE_ATTR(clip, S_IRUGO, pod_wait_for_clip, line6_nop_write);
 static DEVICE_ATTR(device_id, S_IRUGO, pod_get_device_id, line6_nop_write);
 static DEVICE_ATTR(dirty, S_IRUGO, pod_get_dirty, line6_nop_write);
 static DEVICE_ATTR(dump, S_IWUSR | S_IRUGO, pod_get_dump, pod_set_dump);
@@ -1106,7 +1086,6 @@ static int pod_create_files2(struct device *dev)
 {
 	int err;
 
-	CHECK_RETURN(device_create_file(dev, &dev_attr_clip));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_device_id));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_dirty));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_dump));
@@ -1159,7 +1138,6 @@ static int pod_try_init(struct usb_interface *interface,
 	init_waitqueue_head(&pod->tuner_freq.wait);
 	init_waitqueue_head(&pod->tuner_note.wait);
 	init_waitqueue_head(&pod->tuner_pitch.wait);
-	init_waitqueue_head(&pod->clipping.wait);
 
 	memset(pod->param_dirty, 0xff, sizeof(pod->param_dirty));
 
@@ -1250,7 +1228,6 @@ void line6_pod_disconnect(struct usb_interface *interface)
 					       pod->line6.
 					       properties->device_bit, dev);
 
-			device_remove_file(dev, &dev_attr_clip);
 			device_remove_file(dev, &dev_attr_device_id);
 			device_remove_file(dev, &dev_attr_dirty);
 			device_remove_file(dev, &dev_attr_dump);
diff --git a/drivers/staging/line6/pod.h b/drivers/staging/line6/pod.h
index 0f9e83d..fa247b8 100644
--- a/drivers/staging/line6/pod.h
+++ b/drivers/staging/line6/pod.h
@@ -133,11 +133,6 @@ struct usb_line6_pod {
 	struct ValueWait routing;
 
 	/**
-		Wait for audio clipping event.
-	*/
-	struct ValueWait clipping;
-
-	/**
 		Timer for device initializaton.
 	*/
 	struct timer_list startup_timer;
-- 
1.8.0

[Line6linux-devel] [PATCH 06/46] staging: line6: drop dump_buf sysfs attr

[Stefan H. <ste...@gm...>]

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/pod.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/staging/line6/pod.c b/drivers/staging/line6/pod.c
index 56eab0a..c1eb145 100644
--- a/drivers/staging/line6/pod.c
+++ b/drivers/staging/line6/pod.c
@@ -610,42 +610,6 @@ static ssize_t pod_set_system_param_string(struct usb_line6_pod *pod,
 }
 
 /*
-	"read" request on "dump_buf" special file.
-*/
-static ssize_t pod_get_dump_buf(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-	int retval = line6_dump_wait_interruptible(&pod->dumpreq);
-	if (retval < 0)
-		return retval;
-	memcpy(buf, &pod->prog_data_buf, sizeof(pod->prog_data_buf));
-	return sizeof(pod->prog_data_buf);
-}
-
-/*
-	"write" request on "dump_buf" special file.
-*/
-static ssize_t pod_set_dump_buf(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf, size_t count)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-
-	if (count != sizeof(pod->prog_data)) {
-		dev_err(pod->line6.ifcdev,
-			"data block must be exactly %d bytes\n",
-			(int)sizeof(pod->prog_data));
-		return -EINVAL;
-	}
-
-	memcpy(&pod->prog_data_buf, buf, sizeof(pod->prog_data));
-	return sizeof(pod->prog_data);
-}
-
-/*
 	"write" request on "finish" special file.
 */
 static ssize_t pod_set_finish(struct device *dev,
@@ -892,8 +856,6 @@ POD_GET_SYSTEM_PARAM(tuner_pitch, 1);
 
 /* POD special files: */
 static DEVICE_ATTR(device_id, S_IRUGO, pod_get_device_id, line6_nop_write);
-static DEVICE_ATTR(dump_buf, S_IWUSR | S_IRUGO, pod_get_dump_buf,
-		   pod_set_dump_buf);
 static DEVICE_ATTR(finish, S_IWUSR, line6_nop_read, pod_set_finish);
 static DEVICE_ATTR(firmware_version, S_IRUGO, pod_get_firmware_version,
 		   line6_nop_write);
@@ -1004,7 +966,6 @@ static int pod_create_files2(struct device *dev)
 	int err;
 
 	CHECK_RETURN(device_create_file(dev, &dev_attr_device_id));
-	CHECK_RETURN(device_create_file(dev, &dev_attr_dump_buf));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_finish));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_firmware_version));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_midi_postprocess));
@@ -1142,7 +1103,6 @@ void line6_pod_disconnect(struct usb_interface *interface)
 					       properties->device_bit, dev);
 
 			device_remove_file(dev, &dev_attr_device_id);
-			device_remove_file(dev, &dev_attr_dump_buf);
 			device_remove_file(dev, &dev_attr_finish);
 			device_remove_file(dev, &dev_attr_firmware_version);
 			device_remove_file(dev, &dev_attr_midi_postprocess);
-- 
1.8.0

[Line6linux-devel] [PATCH 05/46] staging: line6: drop dump sysfs attr

[Stefan H. <ste...@gm...>]

Once further sysfs attrs have been removed it will also be possible to
drop prog_data and other pieces which we keep for now.

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/pod.c | 60 +++------------------------------------------
 1 file changed, 3 insertions(+), 57 deletions(-)

diff --git a/drivers/staging/line6/pod.c b/drivers/staging/line6/pod.c
index 7c76b65..56eab0a 100644
--- a/drivers/staging/line6/pod.c
+++ b/drivers/staging/line6/pod.c
@@ -138,24 +138,6 @@ static char *pod_alloc_sysex_buffer(struct usb_line6_pod *pod, int code,
 }
 
 /*
-	Send channel dump data to the PODxt Pro.
-*/
-static void pod_dump(struct usb_line6_pod *pod, const unsigned char *data)
-{
-	int size = 1 + sizeof(pod->prog_data);
-	char *sysex = pod_alloc_sysex_buffer(pod, POD_SYSEX_DUMP, size);
-	if (!sysex)
-		return;
-	/* Don't know what this is good for, but PODxt Pro transmits it, so we
-	 * also do... */
-	sysex[SYSEX_DATA_OFS] = 5;
-	memcpy(sysex + SYSEX_DATA_OFS + 1, data, sizeof(pod->prog_data));
-	line6_send_sysex_message(&pod->line6, sysex, size);
-	memcpy(&pod->prog_data, data, sizeof(pod->prog_data));
-	kfree(sysex);
-}
-
-/*
 	Store parameter value in driver memory.
 */
 static void pod_store_parameter(struct usb_line6_pod *pod, int param, int value)
@@ -411,7 +393,9 @@ static ssize_t pod_send_store_command(struct device *dev, const char *buf,
 	if (!sysex)
 		return 0;
 
-	sysex[SYSEX_DATA_OFS] = 5;	/* see pod_dump() */
+	/* Don't know what this is good for, but PODxt Pro transmits it, so we
+	 * also do... */
+	sysex[SYSEX_DATA_OFS] = 5;
 	ret = pod_resolve(buf, block0, block1, sysex + SYSEX_DATA_OFS + 1);
 	if (ret) {
 		kfree(sysex);
@@ -513,41 +497,6 @@ static ssize_t pod_get_name_buf(struct device *dev,
 }
 
 /*
-	"read" request on "dump" special file.
-*/
-static ssize_t pod_get_dump(struct device *dev, struct device_attribute *attr,
-			    char *buf)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-	int retval = line6_dump_wait_interruptible(&pod->dumpreq);
-	if (retval < 0)
-		return retval;
-	memcpy(buf, &pod->prog_data, sizeof(pod->prog_data));
-	return sizeof(pod->prog_data);
-}
-
-/*
-	"write" request on "dump" special file.
-*/
-static ssize_t pod_set_dump(struct device *dev, struct device_attribute *attr,
-			    const char *buf, size_t count)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-
-	if (count != sizeof(pod->prog_data)) {
-		dev_err(pod->line6.ifcdev,
-			"data block must be exactly %d bytes\n",
-			(int)sizeof(pod->prog_data));
-		return -EINVAL;
-	}
-
-	pod_dump(pod, buf);
-	return sizeof(pod->prog_data);
-}
-
-/*
 	Identify system parameters related to the tuner.
 */
 static bool pod_is_tuner(int code)
@@ -943,7 +892,6 @@ POD_GET_SYSTEM_PARAM(tuner_pitch, 1);
 
 /* POD special files: */
 static DEVICE_ATTR(device_id, S_IRUGO, pod_get_device_id, line6_nop_write);
-static DEVICE_ATTR(dump, S_IWUSR | S_IRUGO, pod_get_dump, pod_set_dump);
 static DEVICE_ATTR(dump_buf, S_IWUSR | S_IRUGO, pod_get_dump_buf,
 		   pod_set_dump_buf);
 static DEVICE_ATTR(finish, S_IWUSR, line6_nop_read, pod_set_finish);
@@ -1056,7 +1004,6 @@ static int pod_create_files2(struct device *dev)
 	int err;
 
 	CHECK_RETURN(device_create_file(dev, &dev_attr_device_id));
-	CHECK_RETURN(device_create_file(dev, &dev_attr_dump));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_dump_buf));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_finish));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_firmware_version));
@@ -1195,7 +1142,6 @@ void line6_pod_disconnect(struct usb_interface *interface)
 					       properties->device_bit, dev);
 
 			device_remove_file(dev, &dev_attr_device_id);
-			device_remove_file(dev, &dev_attr_dump);
 			device_remove_file(dev, &dev_attr_dump_buf);
 			device_remove_file(dev, &dev_attr_finish);
 			device_remove_file(dev, &dev_attr_firmware_version);
-- 
1.8.0

[Line6linux-devel] [PATCH 01/46] staging: line6: drop channel sysfs attr

[Stefan H. <ste...@gm...>]

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/pod.c | 51 ---------------------------------------------
 drivers/staging/line6/pod.h |  5 -----
 2 files changed, 56 deletions(-)

diff --git a/drivers/staging/line6/pod.c b/drivers/staging/line6/pod.c
index 4a86f7a..e3e2247 100644
--- a/drivers/staging/line6/pod.c
+++ b/drivers/staging/line6/pod.c
@@ -223,7 +223,6 @@ void line6_pod_process_message(struct usb_line6_pod *pod)
 
 	case LINE6_PROGRAM_CHANGE | LINE6_CHANNEL_DEVICE:
 	case LINE6_PROGRAM_CHANGE | LINE6_CHANNEL_HOST:
-		pod->channel_num = buf[1];
 		pod->dirty = 0;
 		set_bit(POD_CHANNEL_DIRTY, &pod->atomic_flags);
 		line6_dump_request_async(&pod->dumpreq, &pod->line6, 0,
@@ -388,19 +387,6 @@ void line6_pod_midi_postprocess(struct usb_line6_pod *pod, unsigned char *data,
 }
 
 /*
-	Send channel number (i.e., switch to a different sound).
-*/
-static void pod_send_channel(struct usb_line6_pod *pod, u8 value)
-{
-	line6_invalidate_current(&pod->dumpreq);
-
-	if (line6_send_program(&pod->line6, value) == 0)
-		pod->channel_num = value;
-	else
-		line6_dump_finished(&pod->dumpreq);
-}
-
-/*
 	Transmit PODxt Pro control parameter.
 */
 void line6_pod_transmit_parameter(struct usb_line6_pod *pod, int param,
@@ -526,37 +512,6 @@ static ssize_t get_name_generic(struct usb_line6_pod *pod, const char *str,
 }
 
 /*
-	"read" request on "channel" special file.
-*/
-static ssize_t pod_get_channel(struct device *dev,
-			       struct device_attribute *attr, char *buf)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-	return sprintf(buf, "%d\n", pod->channel_num);
-}
-
-/*
-	"write" request on "channel" special file.
-*/
-static ssize_t pod_set_channel(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t count)
-{
-	struct usb_interface *interface = to_usb_interface(dev);
-	struct usb_line6_pod *pod = usb_get_intfdata(interface);
-	u8 value;
-	int ret;
-
-	ret = kstrtou8(buf, 10, &value);
-	if (ret)
-		return ret;
-
-	pod_send_channel(pod, value);
-	return count;
-}
-
-/*
 	"read" request on "name" special file.
 */
 static ssize_t pod_get_name(struct device *dev, struct device_attribute *attr,
@@ -1036,8 +991,6 @@ POD_GET_SYSTEM_PARAM(tuner_pitch, 1);
 #undef GET_SYSTEM_PARAM
 
 /* POD special files: */
-static DEVICE_ATTR(channel, S_IWUSR | S_IRUGO, pod_get_channel,
-		   pod_set_channel);
 static DEVICE_ATTR(clip, S_IRUGO, pod_wait_for_clip, line6_nop_write);
 static DEVICE_ATTR(device_id, S_IRUGO, pod_get_device_id, line6_nop_write);
 static DEVICE_ATTR(dirty, S_IRUGO, pod_get_dirty, line6_nop_write);
@@ -1153,7 +1106,6 @@ static int pod_create_files2(struct device *dev)
 {
 	int err;
 
-	CHECK_RETURN(device_create_file(dev, &dev_attr_channel));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_clip));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_device_id));
 	CHECK_RETURN(device_create_file(dev, &dev_attr_dirty));
@@ -1200,8 +1152,6 @@ static int pod_try_init(struct usb_interface *interface,
 	if ((interface == NULL) || (pod == NULL))
 		return -ENODEV;
 
-	pod->channel_num = 255;
-
 	/* initialize wait queues: */
 	init_waitqueue_head(&pod->monitor_level.wait);
 	init_waitqueue_head(&pod->routing.wait);
@@ -1300,7 +1250,6 @@ void line6_pod_disconnect(struct usb_interface *interface)
 					       pod->line6.
 					       properties->device_bit, dev);
 
-			device_remove_file(dev, &dev_attr_channel);
 			device_remove_file(dev, &dev_attr_clip);
 			device_remove_file(dev, &dev_attr_device_id);
 			device_remove_file(dev, &dev_attr_dirty);
diff --git a/drivers/staging/line6/pod.h b/drivers/staging/line6/pod.h
index 47e0d1a..0f9e83d 100644
--- a/drivers/staging/line6/pod.h
+++ b/drivers/staging/line6/pod.h
@@ -89,11 +89,6 @@ struct usb_line6_pod {
 	struct line6_dump_request dumpreq;
 
 	/**
-		Current program number.
-	*/
-	unsigned char channel_num;
-
-	/**
 		Current program settings.
 	*/
 	struct pod_program prog_data;
-- 
1.8.0

[Line6linux-devel] [PATCH 00/46] staging: line6: drop MIDI state sysfs attrs

[Stefan H. <ste...@gm...>]

Let's put the line6 staging driver on a diet for Thanksgiving! :)

Markus and I have discussed the MIDI processing that currently happens in the
line6 driver.  This stuff really belongs in userspace where a library or
utility can use ALSA MIDI APIs to control the device.  Moving MIDI processing
to userspace has several benefits:

1. The driver only handles PCM and MIDI I/O.  This means less kernel code.

2. A large number of sysfs attributes can be removed.  These attributes exposed
   MIDI state and driver code was necessary to sync and manage this state.

3. It's easier for userspace to control the device, try out new MIDI commands,
   etc when the driver acts as a dumb transport.

This patch series removes the sysfs attributes and then removes the state
behind those attributes.  I have tried to make small incremental changes that
can be reviewed easily.

Testing would be appreciated and especially necessary for POD XT and Variax
devices where the startup process has been modified.  I only have access to a
POD HD300 device which doesn't take these code paths.

The only MIDI processing left are initialization commands and the monitor
volume level, which remains in the kernel because the driver presents an ALSA
volume control.

Stefan Hajnoczi (46):
  staging: line6: drop channel sysfs attr
  staging: line6: drop clip sysfs attr
  staging: line6: drop unused param_dirty bitmap
  staging: line6: drop dirty sysfs attr
  staging: line6: drop dump sysfs attr
  staging: line6: drop dump_buf sysfs attr
  staging: line6: drop monitor_level sysfs attr
  staging: line6: change monitor_level type ValueWait -> int
  staging: line6: drop name sysfs attr
  staging: line6: drop name_buf sysfs attr
  staging: line6: drop retrieve_amp_setup sysfs attr
  staging: line6: drop retrieve_channel sysfs attr
  staging: line6: drop retrieve_effects_setup sysfs attr
  staging: line6: drop store_amp_setup sysfs attr
  staging: line6: drop store_channel sysfs attr
  staging: line6: drop store_effects_setup sysfs attr
  staging: line6: drop routing sysfs attr
  staging: line6: drop tuner_freq sysfs attr
  staging: line6: drop tuner_note sysfs attr
  staging: line6: drop tuner_mute sysfs attr
  staging: line6: drop tuner_pitch sysfs attr
  staging: line6: drop finish sysfs attr
  staging: line6: drop midi_postprocess sysfs attr
  staging: line6: drop midi_mask_receive
  staging: line6: drop midi_mask_transmit
  staging: line6: drop midi_postprocess flag
  staging: line6: drop pod.c raw sysfs attr
  staging: line6: drop tuner param filtering
  staging: line6: drop variax model sysfs attr
  staging: line6: drop variax volume sysfs attr
  staging: line6: drop variax tone sysfs attr
  staging: line6: drop variax name sysfs attr
  staging: line6: drop variax bank sysfs attr
  staging: line6: drop variax dump sysfs attr
  staging: line6: drop variax active sysfs attr
  staging: line6: drop variax guitar sysfs attr
  staging: line6: drop variax raw sysfs attrs
  staging: line6: drop CONFIG_LINE6_USB_RAW
  staging: line6: drop amp/effects dump request triggers
  staging: line6: drop MIDI parameter sysfs attrs
  staging: line6: drop pod prog_data buffers
  staging: line6: drop unused pod atomic_flags field
  staging: line6: drop variax model_data field
  staging: line6: drop dump requests from variax startup
  staging: line6: drop dump requests from pod startup
  staging: line6: drop unused dumprequest code

 drivers/staging/line6/Kconfig       |  11 -
 drivers/staging/line6/Makefile      |   2 -
 drivers/staging/line6/control.c     | 995 ------------------------------------
 drivers/staging/line6/control.h     | 195 -------
 drivers/staging/line6/driver.c      |  20 -
 drivers/staging/line6/dumprequest.c | 135 -----
 drivers/staging/line6/dumprequest.h |  76 ---
 drivers/staging/line6/midi.c        | 120 -----
 drivers/staging/line6/midi.h        |  10 -
 drivers/staging/line6/pod.c         | 846 +-----------------------------
 drivers/staging/line6/pod.h         | 105 +---
 drivers/staging/line6/variax.c      | 480 +----------------
 drivers/staging/line6/variax.h      |  60 ---
 13 files changed, 16 insertions(+), 3039 deletions(-)
 delete mode 100644 drivers/staging/line6/control.c
 delete mode 100644 drivers/staging/line6/control.h
 delete mode 100644 drivers/staging/line6/dumprequest.c
 delete mode 100644 drivers/staging/line6/dumprequest.h

-- 
1.8.0

Re: [Line6linux-devel] line6 out of staging

[Stefan H. <ste...@gm...>]

On Thu, Nov 22, 2012 at 8:57 AM, Laurent Navet <lau...@ya...> wrote:
> Let me know if there's some easy and boring tasks you don't want to waste
> time on. Like fix CodingStyle or so.
> I'm not a kernel guru but can fix minor issues.

I will send out a patch series this weekend that drops a lot of code
(that functionality moves to userspace).  This means coding style
cleanups right now might be wasted on code that gets deleted.

Once I've sent the series I'll also send an email to line6linux-devel
with the todo list that remains.  Maybe you'll find something that you
want to tackle.

I'll CC you on the patches and would appreciate code review and/or
testing.  I only have a POD HD300 so I'm unable to test the POD XT,
Variax, etc code that I've changed.

If you have time to test the patches, please reply to the patch email
thread with Tested-by: Laurent Navet <lau...@ya...> and the
device model that you tested.  Your Tested-by: line becomes part of
the commit history when it gets merged into Linux.

Stefan

[Line6linux-devel] line6 out of staging

[Laurent N. <lau...@ya...>]

Hi guys,

Let me know if there's some easy and boring tasks you don't want to waste time on. Like fix CodingStyle or so.
I'm not a kernel guru but can fix minor issues.

Thank's for your work!
Laurent.

Re: [Line6linux-devel] Status of line6 staging cleanup

[Markus G. <gr...@ic...>]

On Saturday 17 November 2012 01:44:30 Markus Grabner wrote:
> *) The shell is initialized by some hard-coded parameters in "shell.cpp",
> these need to be changed according to your system (it would be useful if the
> Stream class had a static method which lists all available devices such
> that the user can choose one -> TODO).
Just implemented this, run the line6shell without options to get a brief 
documentation. It might not work for all devices, but properly handles my 
PODxt Pro and PODxt Live. Please let me know if it also detects your POD HD!

	Thanks & kind regards,
		Markus

Re: [Line6linux-devel] Status of line6 staging cleanup

[Stefan H. <ste...@gm...>]

On Sat, Nov 17, 2012 at 1:44 AM, Markus Grabner <gr...@ic...> wrote:
> On Friday 16 November 2012 07:05:09 Stefan Hajnoczi wrote:
>> On Thu, Nov 15, 2012 at 11:38 PM, Markus Grabner <gr...@ic...>
> wrote:
>> > Am Donnerstag, 15. November 2012, 08:16:25 schrieb Stefan Hajnoczi:
>> I'm not sure what the best practice is, I guess we'll see when
>> submitting the patches to move the driver out of staging.
>>
>> The only problem I see with sysfs is that it's not clear to me how to
>> control file ownership.  Typically we need the 'audio' group to own
>> the attrs so regular users can read/write them.  sysfs supports
>> chmod(2)/chown(2) but I don't know whether udev(7) rules can apply
>> ownership on sysfs attributes.
> After reviewing the sysfs entries, I noticed that the MIDI mask and MIDI post
> processing flags are the only ones which do not directly translate to a
> message sent to the device, but affect driver state instead. The MIDI
> postprocessing can be moved to user space if necessary, the MIDI masks were
> introduced several years ago to make the driver play nicely with the
> rosegarden MIDI sequencer (I didn't check since then if this is still
> required). Unfortunately ioctl calls seem to be possible only for ALSA PCM
> devices, but not for ALSA MIDI devices, so we might lose this masking
> functionality if we take the ioctl approach, but I don't consider this high
> priority. The TonePort LEDs could be controlled via ioctl calls on the
> TonePort's PCM device, though :-)
>
>> > The CSV table approach discussed in Section 4.4 of the design document
>> > comes close to the self-describing data tables you mention above: all
>> > relevant relations (e.g., the mapping between USB product ids and device
>> > names, or between device parameters and corresponding MIDI byte codes)
>> > are defined and edited in CSV files, which are automatically converted to
>> > corresponding C++ code (enums, tables, maps, etc.) during the build
>> > process. The application can then use these symbols as desired, and
>> > consistently adding a new parameter to all relevant enum lists, tables,
>> > maps, etc., is as easy as adding a single row to a CSV table. The created
>> > files should contain a prominent notice that they were created
>> > automatically, with a reference to the input file (this is not yet
>> > implemented).
>> >
>> > The perl script that does all this magic (parse_data.pl) is actually the
>> > most complex piece of software in the user space code repository at this
>> > time :-)
>> Cool :).  I should try adding POD HD300 support.
> That would actually be a great test for the flexibility of the framework! The
> following steps are required:
>
> *) edit the csv-tables in
> https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased/libline6comm
> according to the HD300. The table "tablecodeparam.csv" maps a MIDI byte code
> (first number in each row after "data") to the corresponding device parameter.
> I don't know if the HD300 behaves similarly (i.e., a single byte is sufficient
> to distinguish the different parameters in the MIDI message). Your USB monitor
> will tell you.
>
> *) If the MIDI message format is different from the PODxt, you need to create
> a class representing the HD300 and implement a suitable
> HD300::midiMessageReceived(). See the files "pod.hpp" and "pod.cpp" for how
> this is done for the PODxt devices.
>
> *) The shell is initialized by some hard-coded parameters in "shell.cpp",
> these need to be changed according to your system (it would be useful if the
> Stream class had a static method which lists all available devices such that
> the user can choose one -> TODO).

Thanks for the pointers.  I'll try it out this week.

Stefan

Re: [Line6linux-devel] Status of line6 staging cleanup

[Markus G. <gr...@ic...>]

On Friday 16 November 2012 07:05:09 Stefan Hajnoczi wrote:
> On Thu, Nov 15, 2012 at 11:38 PM, Markus Grabner <gr...@ic...> 
wrote:
> > Am Donnerstag, 15. November 2012, 08:16:25 schrieb Stefan Hajnoczi:
> I'm not sure what the best practice is, I guess we'll see when
> submitting the patches to move the driver out of staging.
> 
> The only problem I see with sysfs is that it's not clear to me how to
> control file ownership.  Typically we need the 'audio' group to own
> the attrs so regular users can read/write them.  sysfs supports
> chmod(2)/chown(2) but I don't know whether udev(7) rules can apply
> ownership on sysfs attributes.
After reviewing the sysfs entries, I noticed that the MIDI mask and MIDI post 
processing flags are the only ones which do not directly translate to a 
message sent to the device, but affect driver state instead. The MIDI 
postprocessing can be moved to user space if necessary, the MIDI masks were 
introduced several years ago to make the driver play nicely with the 
rosegarden MIDI sequencer (I didn't check since then if this is still 
required). Unfortunately ioctl calls seem to be possible only for ALSA PCM 
devices, but not for ALSA MIDI devices, so we might lose this masking 
functionality if we take the ioctl approach, but I don't consider this high 
priority. The TonePort LEDs could be controlled via ioctl calls on the 
TonePort's PCM device, though :-)

> > The CSV table approach discussed in Section 4.4 of the design document
> > comes close to the self-describing data tables you mention above: all
> > relevant relations (e.g., the mapping between USB product ids and device
> > names, or between device parameters and corresponding MIDI byte codes)
> > are defined and edited in CSV files, which are automatically converted to
> > corresponding C++ code (enums, tables, maps, etc.) during the build
> > process. The application can then use these symbols as desired, and
> > consistently adding a new parameter to all relevant enum lists, tables,
> > maps, etc., is as easy as adding a single row to a CSV table. The created
> > files should contain a prominent notice that they were created
> > automatically, with a reference to the input file (this is not yet
> > implemented).
> > 
> > The perl script that does all this magic (parse_data.pl) is actually the
> > most complex piece of software in the user space code repository at this
> > time :-)
> Cool :).  I should try adding POD HD300 support.
That would actually be a great test for the flexibility of the framework! The 
following steps are required:

*) edit the csv-tables in
https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased/libline6comm
according to the HD300. The table "tablecodeparam.csv" maps a MIDI byte code 
(first number in each row after "data") to the corresponding device parameter. 
I don't know if the HD300 behaves similarly (i.e., a single byte is sufficient 
to distinguish the different parameters in the MIDI message). Your USB monitor 
will tell you.

*) If the MIDI message format is different from the PODxt, you need to create 
a class representing the HD300 and implement a suitable 
HD300::midiMessageReceived(). See the files "pod.hpp" and "pod.cpp" for how 
this is done for the PODxt devices.

*) The shell is initialized by some hard-coded parameters in "shell.cpp", 
these need to be changed according to your system (it would be useful if the 
Stream class had a static method which lists all available devices such that 
the user can choose one -> TODO).

> > As far as I know it is not possible to query a single parameter value from
> > the device (e.g., the current volume), and even if it were, such a query
> > would require a host-device-host roundtrip and suffer from a (small)
> > delay. On the other hand, whenever changing to a different tone on a POD
> > device, the entire settings for the new tone are transmitted to the host
> > (~140 bytes). I therefore think that maintaining the current state is
> > useful here (that's actually how the driver currently responds to read
> > requests on special files representing device parameters, which would
> > also become obsolete in the driver when done by a user space library).
> 
> In a Qt GUI the widgets already keep state - you can ask a slider for
> its current value.  So the application doesn't need the library to
> duplicate that state.
Yes, but I was more thinking of a shell-like application, which doesn't have 
widgets at all. It would still be nice to do something like

PODxtPro> get volume
0.75
PODxtPro>

in the Line6 shell. Here the state must be maintained either by the library or 
by the application. Not sure which one is better...

> I think we're talking about the same thing here.  I mean that the ALSA
> MIDI write call should be non-blocking so that we don't block the main
> loop.
Ok, I just wanted to stress that achieving the non-blocking behaviour by means 
of multi-threading adds unnecessary complexity to the library (and to 
applications using the library).

Please let me knot if you have any questions (probably many) regarding the 
code!

	Kind regards,
		Markus

Re: [Line6linux-devel] Status of line6 staging cleanup

[Stefan H. <ste...@gm...>]

On Thu, Nov 15, 2012 at 11:38 PM, Markus Grabner <gr...@ic...> wrote:
> Am Donnerstag, 15. November 2012, 08:16:25 schrieb Stefan Hajnoczi:
>> On Thu, Nov 15, 2012 at 1:11 AM, Markus Grabner <gr...@ic...>
> wrote:
>> > On Wednesday 14 November 2012 08:45:24 Stefan Hajnoczi wrote:
>> >> The sysfs attributes involve MIDI commands and some state.  If we
>> >> leave this to userspace then the kernel driver can focus on PCM and
>> >> MIDI I/O.  The code will become much smaller and simpler because we
>> >> can drop all the peeking into MIDI buffers and the housekeeping that
>> >> goes along with that.
>> >>
>> >> Letting userspace handle MIDI means that line6linux development
>> >> becomes accessible to a wider group of developers - people not
>> >> comfortable with C or driver hacking.  It encourages people to explore
>> >> their devices and contribute code.
>> >
>> > I agree with all of that. Let me point to a document which I started to
>> > write a year ago when we first briefly discussed this:
>> >
>> > https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qt
>> > based/doc/design.tex
>> >
>> > The sysfs attributes are ugly, but very convenient for development and
>> > testing. I don't want to remove them without having a similarly convenient
>> > replacement, that's why I started to write some code according to the
>> > ideas
>> > discussed in the abovementioned document. Feel free to comment on this!
>> >
>> > One particular question is how to best control the driver's behaviour
>> > (e.g., the midi masks) from user mode. I can imagine at least two
>> > methods: *) using ioctl() calls
>> > *) defining a virtual MIDI device for communication with the driver
>> >
>> > Are there more options, and what would be considered best practise in this
>> > case?
>>
>> Either ioctl() or sysfs is okay.  ioctl() is very easy for C/C++
>> applications to use if they already have a file descriptor.  For
>> human-readable data sysfs is nicer since it can be accessed easily
>> from a shell session.  The challenge with sysfs is locating the device
>> directory, but this could be done given the ALSA device numbers.
> As far as I remember, it was Linus himself who recommended the special files
> in the Line6 driver to be writable only by root :-), so this would be too
> restrictive for most use cases. If it is accepted practise to define ioctl()
> calls for this purpose, this is probably the best solution once we have user-
> space tools to access Line6 devices. This would replace a command like
>
> echo 4 > /sys/bus/usb/drivers/line6usb/7-2:1.0/midi_mask_transmit
>
> by something like
>
> /usr/bin/line6 -d PODxtPro set midi_mask_transmit 4

I'm not sure what the best practice is, I guess we'll see when
submitting the patches to move the driver out of staging.

The only problem I see with sysfs is that it's not clear to me how to
control file ownership.  Typically we need the 'audio' group to own
the attrs so regular users can read/write them.  sysfs supports
chmod(2)/chown(2) but I don't know whether udev(7) rules can apply
ownership on sysfs attributes.

>> The design document you posted looks sane and well thought out.  I
>> think the main "meat" of the library will be the data tables for each
>> supported device.  Ideally the library code would not hard-code names
>> (e.g. amp1_gain, wah_position) but instead use self-describing data
>> tables.  The advantage here is that developers can easily add new
>> tweak parameters by modifying the data tables.  The only piece of
>> software that needs to understand the parameter names is the GUI, and
>> even there we could try to use a data-driven approach.
>>
>> In other words, the library doesn't need enums for all possible
>> parameters.  Instead you could say line6_set_param(dev,
>> "master_volume", 0x40), which looks up "master_volume" in the device's
>> data table.  That produces a MIDI message (PC, CC, or SysEx).
> I actually prefer symbols over strings for this purpose for several reasons:
> *) a typo in a string doesn't get noticed at compile time
> *) symbols can be used in switch/case statements
> *) comparing an integer variable with a symbolic constant gives cleaner code
> than a string comparison (using the "==" operator instead of calling "strcmp")
> *) an integer is lightweight compared to a string, both in terms of memory
> consumption and performance

Since you have code working today let's go with the enum approach.

>
> The CSV table approach discussed in Section 4.4 of the design document comes
> close to the self-describing data tables you mention above: all relevant
> relations (e.g., the mapping between USB product ids and device names, or
> between device parameters and corresponding MIDI byte codes) are defined and
> edited in CSV files, which are automatically converted to corresponding C++
> code (enums, tables, maps, etc.) during the build process. The application can
> then use these symbols as desired, and consistently adding a new parameter to
> all relevant enum lists, tables, maps, etc., is as easy as adding a single row
> to a CSV table. The created files should contain a prominent notice that they
> were created automatically, with a reference to the input file (this is not
> yet implemented).
>
> The perl script that does all this magic (parse_data.pl) is actually the most
> complex piece of software in the user space code repository at this time :-)

Cool :).  I should try adding POD HD300 support.

>> When the device sends MIDI messages to the host in response to the wah
>> pedal, for example, the library uses the data table to translate from
>> the MIDI message to event_param_changed(dev, "wah_position", 0x7f).
> That's basically how it is (partially) implemented, except that a symbolic
> constant is used instead of a string, and that the parameter id and the value
> are collected in an "Action" class. This is yet missing from the design
> document, the basic idea is to have a (small) set of action types (such as
> "set a continuous parameter value", "set a boolean parameter value", etc.).
> These are device-independent (though certainly not all devices support all
> parameters), and it's the library's responsibility to translate between these
> and the corrsponding device-dependent MIDI byte codes when transmitting or
> receiving data.
>
>> The design document doesn't cover whether to make the library stateful
>> or not.  A stateless library simply transmits and receives events on
>> behalf of the application.  A stateful library keeps a "current value"
>> stored for every parameter in the data table so that the application
>> can retrieve it quickly.  My feeling is that a stateless library is a
>> good thing but there is a risk that applications will duplicate code,
>> so we'd have to watch out if we find ourselves reimplementing the same
>> state code in multiple applications.
> As far as I know it is not possible to query a single parameter value from the
> device (e.g., the current volume), and even if it were, such a query would
> require a host-device-host roundtrip and suffer from a (small) delay. On the
> other hand, whenever changing to a different tone on a POD device, the entire
> settings for the new tone are transmitted to the host (~140 bytes). I
> therefore think that maintaining the current state is useful here (that's
> actually how the driver currently responds to read requests on special files
> representing device parameters, which would also become obsolete in the driver
> when done by a user space library).

In a Qt GUI the widgets already keep state - you can ask a slider for
its current value.  So the application doesn't need the library to
duplicate that state.

BTW I have begun removing the MIDI sysfs attributes from pod.c.  I'll
send patches next week.

>> A small detail that came to mind: transmitting MIDI should be
>> asynchronous just like receiving MIDI is.  The reason for this is that
>> sending large commands (e.g. restoring saved presets) should not block
>> the GUI.
> After the headaches we had with a "true" asynchronous approach in our first
> attempt, I think it's less error-prone to use the select() mechanism provided
> by most UI libraries. In Qt, you can request notification when data are ready
> for reading (or writing data has completed), by connecting signals from the Qt
> device classes with corresponding slots in your application code. It is
> probably sufficient to use non-blocking I/O and react on the completion
> notification (e.g., by displaying a "process completed" dialog). If a progress
> bar should be displayed, a large message should be split into smaller pieces
> (e.g., one per preset) to allow for proper upgrade of the progress bar, but
> the Qt main loop frequently get the opportunity to process user interface
> events such that the GUI remains responsive. I'm not familiar with GTK, but I
> think it's similar there.

I think we're talking about the same thing here.  I mean that the ALSA
MIDI write call should be non-blocking so that we don't block the main
loop.

>
>> Is there code to implement this design?
> Yes, it's under
>
> https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased
>
> The name "qtbased" is actually a misnomer, but the idea to clearly separate
> the code related to a particular user interface library (Section 3.3.2 of the
> design document) only gradually evolved while working on this branch, and I
> simply didn't want to rename it again.
>
> Be aware that the code is very experimental, poorly documented, and quite a
> mess. Nevertheless, the line6shell at least displays clear text descriptions
> of events received from the device by making use of the auto-generated tables,
> i.e., without any hard-coded parameter name in the code. Have fun exploring
> the code :-)

Thanks!

Stefan

Re: [Line6linux-devel] [PATCH 1/8] staging: line6: wrap >80 char lines in capture.c

[Joe P. <jo...@pe...>]

On Fri, 2012-11-16 at 00:43 +0100, Markus Grabner wrote:
> On Thursday 15 November 2012 14:12:31 Joe Perches wrote:
> > On Thu, 2012-11-15 at 22:03 +0100, Markus Grabner wrote:
> > > Am Mittwoch, 14. November 2012, 17:33:05 schrieb Dan Carpenter:
> > > > The reason this is hitting the 80 character limit is because
> > > > "LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM" is 35 characters long.  It
> > > > isn't even clear from the name what it holds.  It's just a very crap
> > > > name.
> > > 
> > > Please refer to the file pcm.h for a detailed documentation of this and
> > > similar names (in fact, the documentation explains the LINE6_BIT_PCM_*
> > > names instead, but I bevlieve the correspondence is obvious). It's hard
> > > to define a shorter name which is at the same time descriptive,
> > > consistent, and not to be confused with related names.
> > > 
> > > Should such documentation be moved to a separate file (e.g.,
> > > "Documentation/sound/alsa/line6usb.txt")?
> > 
> > Documenting poor naming choices doesn't make it better.
> Yes, but the documentation might help understanding why a particular naming 
> was chosen and that it might not be as poor as it seemed at first sight. I 
> assume that you are aware of the meaning of the LINE6_INDEX_PCM_* symbols (and 
> of the issues that were fixed by introducing them), so which naming scheme 

Hi Markus

Dunno why they were introduced, but I think
several things could be shorter as to me
none of these longish L6_BIT(foo) elements
means much and I'd need to read the code
to figure them out anyway.

the LINE6_ prefix is excessive,
L6_ would probably be fine.
(and that goes for all the functions too)

CAPTURE/RECORD could be RD/WR
MONITOR could be MON
IMPULSE could be IRM

BUFFER could be I
STREAM could be O
(or the other way 'round)

etc..., cheers, Joe

Re: [Line6linux-devel] [PATCH 1/8] staging: line6: wrap >80 char lines in capture.c

[Markus G. <gr...@ic...>]

On Thursday 15 November 2012 14:12:31 Joe Perches wrote:
> On Thu, 2012-11-15 at 22:03 +0100, Markus Grabner wrote:
> > Am Mittwoch, 14. November 2012, 17:33:05 schrieb Dan Carpenter:
> > > The reason this is hitting the 80 character limit is because
> > > "LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM" is 35 characters long.  It
> > > isn't even clear from the name what it holds.  It's just a very crap
> > > name.
> > 
> > Please refer to the file pcm.h for a detailed documentation of this and
> > similar names (in fact, the documentation explains the LINE6_BIT_PCM_*
> > names instead, but I bevlieve the correspondence is obvious). It's hard
> > to define a shorter name which is at the same time descriptive,
> > consistent, and not to be confused with related names.
> > 
> > Should such documentation be moved to a separate file (e.g.,
> > "Documentation/sound/alsa/line6usb.txt")?
> 
> Documenting poor naming choices doesn't make it better.
Yes, but the documentation might help understanding why a particular naming 
was chosen and that it might not be as poor as it seemed at first sight. I 
assume that you are aware of the meaning of the LINE6_INDEX_PCM_* symbols (and 
of the issues that were fixed by introducing them), so which naming scheme 
would you propose instead?

	Kind regards,
		Markus

Re: [Line6linux-devel] Status of line6 staging cleanup

[Markus G. <gr...@ic...>]

Am Donnerstag, 15. November 2012, 08:16:25 schrieb Stefan Hajnoczi:
> On Thu, Nov 15, 2012 at 1:11 AM, Markus Grabner <gr...@ic...> 
wrote:
> > On Wednesday 14 November 2012 08:45:24 Stefan Hajnoczi wrote:
> >> The sysfs attributes involve MIDI commands and some state.  If we
> >> leave this to userspace then the kernel driver can focus on PCM and
> >> MIDI I/O.  The code will become much smaller and simpler because we
> >> can drop all the peeking into MIDI buffers and the housekeeping that
> >> goes along with that.
> >> 
> >> Letting userspace handle MIDI means that line6linux development
> >> becomes accessible to a wider group of developers - people not
> >> comfortable with C or driver hacking.  It encourages people to explore
> >> their devices and contribute code.
> > 
> > I agree with all of that. Let me point to a document which I started to
> > write a year ago when we first briefly discussed this:
> > 
> > https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qt
> > based/doc/design.tex
> > 
> > The sysfs attributes are ugly, but very convenient for development and
> > testing. I don't want to remove them without having a similarly convenient
> > replacement, that's why I started to write some code according to the
> > ideas
> > discussed in the abovementioned document. Feel free to comment on this!
> > 
> > One particular question is how to best control the driver's behaviour
> > (e.g., the midi masks) from user mode. I can imagine at least two
> > methods: *) using ioctl() calls
> > *) defining a virtual MIDI device for communication with the driver
> > 
> > Are there more options, and what would be considered best practise in this
> > case?
> 
> Either ioctl() or sysfs is okay.  ioctl() is very easy for C/C++
> applications to use if they already have a file descriptor.  For
> human-readable data sysfs is nicer since it can be accessed easily
> from a shell session.  The challenge with sysfs is locating the device
> directory, but this could be done given the ALSA device numbers.
As far as I remember, it was Linus himself who recommended the special files 
in the Line6 driver to be writable only by root :-), so this would be too 
restrictive for most use cases. If it is accepted practise to define ioctl() 
calls for this purpose, this is probably the best solution once we have user-
space tools to access Line6 devices. This would replace a command like

echo 4 > /sys/bus/usb/drivers/line6usb/7-2:1.0/midi_mask_transmit

by something like

/usr/bin/line6 -d PODxtPro set midi_mask_transmit 4

> The design document you posted looks sane and well thought out.  I
> think the main "meat" of the library will be the data tables for each
> supported device.  Ideally the library code would not hard-code names
> (e.g. amp1_gain, wah_position) but instead use self-describing data
> tables.  The advantage here is that developers can easily add new
> tweak parameters by modifying the data tables.  The only piece of
> software that needs to understand the parameter names is the GUI, and
> even there we could try to use a data-driven approach.
> 
> In other words, the library doesn't need enums for all possible
> parameters.  Instead you could say line6_set_param(dev,
> "master_volume", 0x40), which looks up "master_volume" in the device's
> data table.  That produces a MIDI message (PC, CC, or SysEx).
I actually prefer symbols over strings for this purpose for several reasons:
*) a typo in a string doesn't get noticed at compile time
*) symbols can be used in switch/case statements
*) comparing an integer variable with a symbolic constant gives cleaner code 
than a string comparison (using the "==" operator instead of calling "strcmp")
*) an integer is lightweight compared to a string, both in terms of memory 
consumption and performance

The CSV table approach discussed in Section 4.4 of the design document comes 
close to the self-describing data tables you mention above: all relevant 
relations (e.g., the mapping between USB product ids and device names, or 
between device parameters and corresponding MIDI byte codes) are defined and 
edited in CSV files, which are automatically converted to corresponding C++ 
code (enums, tables, maps, etc.) during the build process. The application can 
then use these symbols as desired, and consistently adding a new parameter to 
all relevant enum lists, tables, maps, etc., is as easy as adding a single row 
to a CSV table. The created files should contain a prominent notice that they 
were created automatically, with a reference to the input file (this is not 
yet implemented).

The perl script that does all this magic (parse_data.pl) is actually the most 
complex piece of software in the user space code repository at this time :-)

> When the device sends MIDI messages to the host in response to the wah
> pedal, for example, the library uses the data table to translate from
> the MIDI message to event_param_changed(dev, "wah_position", 0x7f).
That's basically how it is (partially) implemented, except that a symbolic 
constant is used instead of a string, and that the parameter id and the value 
are collected in an "Action" class. This is yet missing from the design 
document, the basic idea is to have a (small) set of action types (such as 
"set a continuous parameter value", "set a boolean parameter value", etc.). 
These are device-independent (though certainly not all devices support all 
parameters), and it's the library's responsibility to translate between these 
and the corrsponding device-dependent MIDI byte codes when transmitting or 
receiving data.

> The design document doesn't cover whether to make the library stateful
> or not.  A stateless library simply transmits and receives events on
> behalf of the application.  A stateful library keeps a "current value"
> stored for every parameter in the data table so that the application
> can retrieve it quickly.  My feeling is that a stateless library is a
> good thing but there is a risk that applications will duplicate code,
> so we'd have to watch out if we find ourselves reimplementing the same
> state code in multiple applications.
As far as I know it is not possible to query a single parameter value from the 
device (e.g., the current volume), and even if it were, such a query would 
require a host-device-host roundtrip and suffer from a (small) delay. On the 
other hand, whenever changing to a different tone on a POD device, the entire 
settings for the new tone are transmitted to the host (~140 bytes). I 
therefore think that maintaining the current state is useful here (that's 
actually how the driver currently responds to read requests on special files 
representing device parameters, which would also become obsolete in the driver 
when done by a user space library).

> A small detail that came to mind: transmitting MIDI should be
> asynchronous just like receiving MIDI is.  The reason for this is that
> sending large commands (e.g. restoring saved presets) should not block
> the GUI.
After the headaches we had with a "true" asynchronous approach in our first 
attempt, I think it's less error-prone to use the select() mechanism provided 
by most UI libraries. In Qt, you can request notification when data are ready 
for reading (or writing data has completed), by connecting signals from the Qt 
device classes with corresponding slots in your application code. It is 
probably sufficient to use non-blocking I/O and react on the completion 
notification (e.g., by displaying a "process completed" dialog). If a progress 
bar should be displayed, a large message should be split into smaller pieces 
(e.g., one per preset) to allow for proper upgrade of the progress bar, but 
the Qt main loop frequently get the opportunity to process user interface 
events such that the GUI remains responsive. I'm not familiar with GTK, but I 
think it's similar there.

> Is there code to implement this design?
Yes, it's under

https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased

The name "qtbased" is actually a misnomer, but the idea to clearly separate 
the code related to a particular user interface library (Section 3.3.2 of the 
design document) only gradually evolved while working on this branch, and I 
simply didn't want to rename it again.

Be aware that the code is very experimental, poorly documented, and quite a 
mess. Nevertheless, the line6shell at least displays clear text descriptions 
of events received from the device by making use of the auto-generated tables, 
i.e., without any hard-coded parameter name in the code. Have fun exploring 
the code :-)

	Kind regards,
		Markus

Re: [Line6linux-devel] [PATCH 1/8] staging: line6: wrap >80 char lines in capture.c

[Joe P. <jo...@pe...>]

On Thu, 2012-11-15 at 22:03 +0100, Markus Grabner wrote:
> Am Mittwoch, 14. November 2012, 17:33:05 schrieb Dan Carpenter:
> > The reason this is hitting the 80 character limit is because
> > "LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM" is 35 characters long.  It
> > isn't even clear from the name what it holds.  It's just a very crap
> > name.
> Please refer to the file pcm.h for a detailed documentation of this and 
> similar names (in fact, the documentation explains the LINE6_BIT_PCM_* names 
> instead, but I bevlieve the correspondence is obvious). It's hard to define a 
> shorter name which is at the same time descriptive, consistent, and not to be 
> confused with related names.
> 
> Should such documentation be moved to a separate file (e.g., 
> "Documentation/sound/alsa/line6usb.txt")?

Documenting poor naming choices doesn't make it better.

Re: [Line6linux-devel] [PATCH 1/8] staging: line6: wrap >80 char lines in capture.c

[Dan C. <dan...@or...>]

On Thu, Nov 15, 2012 at 10:03:27PM +0100, Markus Grabner wrote:
> Am Mittwoch, 14. November 2012, 17:33:05 schrieb Dan Carpenter:
> > On Sun, Nov 11, 2012 at 01:24:39PM +0100, Stefan Hajnoczi wrote:
> > > Signed-off-by: Stefan Hajnoczi <ste...@gm...>
> > > ---
> > > 
> > >  drivers/staging/line6/capture.c | 13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/staging/line6/capture.c
> > > b/drivers/staging/line6/capture.c index c85c5b6..389c41f 100644
> > > --- a/drivers/staging/line6/capture.c
> > > +++ b/drivers/staging/line6/capture.c
> > > @@ -256,8 +256,8 @@ static void audio_in_callback(struct urb *urb)
> > > 
> > >  #ifdef CONFIG_LINE6_USB_IMPULSE_RESPONSE
> > >  
> > >  		if (!(line6pcm->flags & LINE6_BITS_PCM_IMPULSE))
> > >  
> > >  #endif
> > > 
> > > -			if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM, &line6pcm-
> >flags)
> > > -			    && (fsize > 0))
> > > +			if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM,
> > 
> > The reason this is hitting the 80 character limit is because
> > "LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM" is 35 characters long.  It
> > isn't even clear from the name what it holds.  It's just a very crap
> > name.
> Please refer to the file pcm.h for a detailed documentation of this and 
> similar names (in fact, the documentation explains the LINE6_BIT_PCM_* names 
> instead, but I bevlieve the correspondence is obvious). It's hard to define a 
> shorter name which is at the same time descriptive, consistent, and not to be 
> confused with related names.
> 
> Should such documentation be moved to a separate file (e.g., 
> "Documentation/sound/alsa/line6usb.txt")?

The documentation is pretty good actually and it belongs where it
is.  But 35 characters is just tooooooooooooooooooooooooooooooooooo
long.

To me the word "INDEX_" is confusing because what are we indexing?
In this context it means the the variable is a bit flag.  That was
obvious because we were calling test_bit().

I think we could drop the PCM_ as well, because why do we need that?

We could change the LINE6 to L6.

			if (test_bit(L6_ALSA_CAPTURE_STREAM, &line6pcm - flags)

It fits!

regards,
dan carpenter

Re: [Line6linux-devel] [PATCH 1/8] staging: line6: wrap >80 char lines in capture.c

[Markus G. <gr...@ic...>]

Am Mittwoch, 14. November 2012, 17:33:05 schrieb Dan Carpenter:
> On Sun, Nov 11, 2012 at 01:24:39PM +0100, Stefan Hajnoczi wrote:
> > Signed-off-by: Stefan Hajnoczi <ste...@gm...>
> > ---
> > 
> >  drivers/staging/line6/capture.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/line6/capture.c
> > b/drivers/staging/line6/capture.c index c85c5b6..389c41f 100644
> > --- a/drivers/staging/line6/capture.c
> > +++ b/drivers/staging/line6/capture.c
> > @@ -256,8 +256,8 @@ static void audio_in_callback(struct urb *urb)
> > 
> >  #ifdef CONFIG_LINE6_USB_IMPULSE_RESPONSE
> >  
> >  		if (!(line6pcm->flags & LINE6_BITS_PCM_IMPULSE))
> >  
> >  #endif
> > 
> > -			if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM, &line6pcm-
>flags)
> > -			    && (fsize > 0))
> > +			if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM,
> 
> The reason this is hitting the 80 character limit is because
> "LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM" is 35 characters long.  It
> isn't even clear from the name what it holds.  It's just a very crap
> name.
Please refer to the file pcm.h for a detailed documentation of this and 
similar names (in fact, the documentation explains the LINE6_BIT_PCM_* names 
instead, but I bevlieve the correspondence is obvious). It's hard to define a 
shorter name which is at the same time descriptive, consistent, and not to be 
confused with related names.

Should such documentation be moved to a separate file (e.g., 
"Documentation/sound/alsa/line6usb.txt")?

	Kind regards,
		Markus

Re: [Line6linux-devel] Status of line6 staging cleanup

[Stefan H. <ste...@gm...>]

On Thu, Nov 15, 2012 at 1:11 AM, Markus Grabner <gr...@ic...> wrote:
> On Wednesday 14 November 2012 08:45:24 Stefan Hajnoczi wrote:
>> The sysfs attributes involve MIDI commands and some state.  If we
>> leave this to userspace then the kernel driver can focus on PCM and
>> MIDI I/O.  The code will become much smaller and simpler because we
>> can drop all the peeking into MIDI buffers and the housekeeping that
>> goes along with that.
>>
>> Letting userspace handle MIDI means that line6linux development
>> becomes accessible to a wider group of developers - people not
>> comfortable with C or driver hacking.  It encourages people to explore
>> their devices and contribute code.
> I agree with all of that. Let me point to a document which I started to write
> a year ago when we first briefly discussed this:
>
> https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased/doc/design.tex
>
> The sysfs attributes are ugly, but very convenient for development and
> testing. I don't want to remove them without having a similarly convenient
> replacement, that's why I started to write some code according to the ideas
> discussed in the abovementioned document. Feel free to comment on this!
>
> One particular question is how to best control the driver's behaviour (e.g.,
> the midi masks) from user mode. I can imagine at least two methods:
> *) using ioctl() calls
> *) defining a virtual MIDI device for communication with the driver
>
> Are there more options, and what would be considered best practise in this
> case?

Either ioctl() or sysfs is okay.  ioctl() is very easy for C/C++
applications to use if they already have a file descriptor.  For
human-readable data sysfs is nicer since it can be accessed easily
from a shell session.  The challenge with sysfs is locating the device
directory, but this could be done given the ALSA device numbers.

The design document you posted looks sane and well thought out.  I
think the main "meat" of the library will be the data tables for each
supported device.  Ideally the library code would not hard-code names
(e.g. amp1_gain, wah_position) but instead use self-describing data
tables.  The advantage here is that developers can easily add new
tweak parameters by modifying the data tables.  The only piece of
software that needs to understand the parameter names is the GUI, and
even there we could try to use a data-driven approach.

In other words, the library doesn't need enums for all possible
parameters.  Instead you could say line6_set_param(dev,
"master_volume", 0x40), which looks up "master_volume" in the device's
data table.  That produces a MIDI message (PC, CC, or SysEx).

When the device sends MIDI messages to the host in response to the wah
pedal, for example, the library uses the data table to translate from
the MIDI message to event_param_changed(dev, "wah_position", 0x7f).

The design document doesn't cover whether to make the library stateful
or not.  A stateless library simply transmits and receives events on
behalf of the application.  A stateful library keeps a "current value"
stored for every parameter in the data table so that the application
can retrieve it quickly.  My feeling is that a stateless library is a
good thing but there is a risk that applications will duplicate code,
so we'd have to watch out if we find ourselves reimplementing the same
state code in multiple applications.

A small detail that came to mind: transmitting MIDI should be
asynchronous just like receiving MIDI is.  The reason for this is that
sending large commands (e.g. restoring saved presets) should not block
the GUI.

Is there code to implement this design?

Stefan

Re: [Line6linux-devel] Status of line6 staging cleanup

[Markus G. <gr...@ic...>]

On Wednesday 14 November 2012 08:45:24 Stefan Hajnoczi wrote:
> I have begun cleaning up the staging driver so it can be merged by the
> ALSA maintainers.
> 
> The first level of cleanups is to satisfy checkpatch.pl.  These are
> mostly trivial changes and don't require much discussion.
> 
> Next I've begun trimming redundant code.  Some of the debugging
> Kconfig options appear to be superceded by generic dumping tools like
> dyndbg, usbmon, amidi/aseqdump so I am sending patches to drop them.
> If you feel they should be kept please respond to the patches.
Ok, I'll take a look.

> The real issue is what to do with all the sysfs attributes.  They
> cannot be merged without further work, but I feel it is a mistake to
> have them in the first place:
> 
> The sysfs attributes involve MIDI commands and some state.  If we
> leave this to userspace then the kernel driver can focus on PCM and
> MIDI I/O.  The code will become much smaller and simpler because we
> can drop all the peeking into MIDI buffers and the housekeeping that
> goes along with that.
> 
> Letting userspace handle MIDI means that line6linux development
> becomes accessible to a wider group of developers - people not
> comfortable with C or driver hacking.  It encourages people to explore
> their devices and contribute code.
I agree with all of that. Let me point to a document which I started to write 
a year ago when we first briefly discussed this:

https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased/doc/design.tex

The sysfs attributes are ugly, but very convenient for development and 
testing. I don't want to remove them without having a similarly convenient 
replacement, that's why I started to write some code according to the ideas 
discussed in the abovementioned document. Feel free to comment on this!

One particular question is how to best control the driver's behaviour (e.g., 
the midi masks) from user mode. I can imagine at least two methods:
*) using ioctl() calls
*) defining a virtual MIDI device for communication with the driver

Are there more options, and what would be considered best practise in this 
case?

> That said, I don't want to break the userspace tools that you have
> written.  Can you point me to userspace tools that would need to be
> fixed before we drop sysfs attributes?
Some of the scripts under 
https://line6linux.svn.sourceforge.net/svnroot/line6linux/driver/trunk (in 
particular the test suite) make use of the sysfs attributes, but I'm prepared 
to rewrite the relevant parts once a better method is available :-)

	Kind regards,
		Markus


-- 
Markus Grabner
Institute for Computer Graphics and Vision
Graz University of Technology, Inffeldgasse 16a/II, 8010 Graz, Austria
WWW: http://www.icg.tugraz.at/Members/grabner

Re: [Line6linux-devel] [PATCH 1/3] staging: line6: replace DEBUG_MESSAGES() with dev_dbg()

[Dan C. <dan...@or...>]

On Sun, Nov 11, 2012 at 01:52:24PM +0100, Stefan Hajnoczi wrote:
> +					dev_dbg(pod->line6.ifcdev,
> +						"wrong size of channel dump message (%d instead of %d)\n",
> +						pod->line6.message_length,
> +						(int)sizeof(pod->prog_data) +
> +						7);

Better to get rid of the cast.  You're already over the 80 character
limit so putting the "7);" on the line before is ok.

regards,
dan carpenter

Re: [Line6linux-devel] [PATCH 1/8] staging: line6: wrap >80 char lines in capture.c

[Dan C. <dan...@or...>]

On Sun, Nov 11, 2012 at 01:24:39PM +0100, Stefan Hajnoczi wrote:
> Signed-off-by: Stefan Hajnoczi <ste...@gm...>
> ---
>  drivers/staging/line6/capture.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/line6/capture.c b/drivers/staging/line6/capture.c
> index c85c5b6..389c41f 100644
> --- a/drivers/staging/line6/capture.c
> +++ b/drivers/staging/line6/capture.c
> @@ -256,8 +256,8 @@ static void audio_in_callback(struct urb *urb)
>  #ifdef CONFIG_LINE6_USB_IMPULSE_RESPONSE
>  		if (!(line6pcm->flags & LINE6_BITS_PCM_IMPULSE))
>  #endif
> -			if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM, &line6pcm->flags)
> -			    && (fsize > 0))
> +			if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM,

The reason this is hitting the 80 character limit is because
"LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM" is 35 characters long.  It
isn't even clear from the name what it holds.  It's just a very crap
name.

regards,
dan carpenter

[Line6linux-devel] [PATCH 3/6] staging: line6: drop unused CONFIG_LINE6_USB_DUMP_CTRL

[Stefan H. <ste...@gm...>]

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/Kconfig | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/line6/Kconfig b/drivers/staging/line6/Kconfig
index a5ded12..2101799 100644
--- a/drivers/staging/line6/Kconfig
+++ b/drivers/staging/line6/Kconfig
@@ -23,15 +23,6 @@ menuconfig LINE6_USB
 
 if LINE6_USB
 
-config LINE6_USB_DUMP_CTRL
-	bool "dump control messages"
-	default n
-	help
-	  Say Y here to write control messages sent to and received from
-	  Line6 devices to the syslog.
-
-	  If unsure, say N.
-
 config LINE6_USB_DUMP_MIDI
 	bool "dump MIDI messages"
 	default n
-- 
1.8.0

[Line6linux-devel] [PATCH 6/6] staging: line6: drop unused CONFIG_LINE6_USB_DUMP_MIDI

[Stefan H. <ste...@gm...>]

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/Kconfig | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/line6/Kconfig b/drivers/staging/line6/Kconfig
index 2101799..80a7202 100644
--- a/drivers/staging/line6/Kconfig
+++ b/drivers/staging/line6/Kconfig
@@ -23,15 +23,6 @@ menuconfig LINE6_USB
 
 if LINE6_USB
 
-config LINE6_USB_DUMP_MIDI
-	bool "dump MIDI messages"
-	default n
-	help
-	  Say Y here to write MIDI messages sent to and received from
-	  Line6 devices to the syslog.
-
-	  If unsure, say N.
-
 config LINE6_USB_DUMP_PCM
 	bool "dump PCM data"
 	default n
-- 
1.8.0

[Line6linux-devel] [PATCH 4/6] staging: line6: drop MIDI dumping code

[Stefan H. <ste...@gm...>]

ALSA amidi(1) and aseqdump(1) can be used to dump MIDI instead of
manually dumping MIDI messages in the driver.  The advantage of using
these existing tools is that can be used at run-time rather than
compile-time.

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/driver.c | 3 ---
 drivers/staging/line6/midi.c   | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/staging/line6/driver.c b/drivers/staging/line6/driver.c
index fda92d1..0bc838d 100644
--- a/drivers/staging/line6/driver.c
+++ b/drivers/staging/line6/driver.c
@@ -402,9 +402,6 @@ static void line6_data_received(struct urb *urb)
 			continue;
 
 		line6->message_length = done;
-#ifdef CONFIG_LINE6_USB_DUMP_MIDI
-		line6_write_hexdump(line6, 'r', line6->buffer_message, done);
-#endif
 		line6_midi_receive(line6, line6->buffer_message, done);
 
 		switch (line6->usbdev->descriptor.idProduct) {
diff --git a/drivers/staging/line6/midi.c b/drivers/staging/line6/midi.c
index 348d425..c8e099b 100644
--- a/drivers/staging/line6/midi.c
+++ b/drivers/staging/line6/midi.c
@@ -59,9 +59,6 @@ static void line6_midi_transmit(struct snd_rawmidi_substream *substream)
 		if (done == 0)
 			break;
 
-#ifdef CONFIG_LINE6_USB_DUMP_MIDI
-		line6_write_hexdump(line6, 's', chunk, done);
-#endif
 		line6_midibuf_write(mb, chunk, done);
 		snd_rawmidi_transmit_ack(substream, done);
 	}
-- 
1.8.0

[Line6linux-devel] [PATCH 1/6] staging: line6: drop control URB dumping code

[Stefan H. <ste...@gm...>]

The usbmon feature should be used instead of manually dumping control
URBs.  There are a few advantages to using usbmon:

 * Can be turned on/off at runtime
 * Provides full USB-level traffic
 * tcpdump and wireshark support for powerful analysis
 * No driver-specific code is required

Signed-off-by: Stefan Hajnoczi <ste...@gm...>
---
 drivers/staging/line6/driver.c | 36 ------------------------------------
 drivers/staging/line6/midi.c   |  3 ---
 2 files changed, 39 deletions(-)

diff --git a/drivers/staging/line6/driver.c b/drivers/staging/line6/driver.c
index 571f2ce..fda92d1 100644
--- a/drivers/staging/line6/driver.c
+++ b/drivers/staging/line6/driver.c
@@ -177,22 +177,6 @@ void line6_write_hexdump(struct usb_line6 *line6, char dir,
 }
 #endif
 
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-/*
-	Dump URB data to syslog.
-*/
-static void line6_dump_urb(struct urb *urb)
-{
-	struct usb_line6 *line6 = (struct usb_line6 *)urb->context;
-
-	if (urb->status < 0)
-		return;
-
-	line6_write_hexdump(line6, 'R', (unsigned char *)urb->transfer_buffer,
-			    urb->actual_length);
-}
-#endif
-
 /*
 	Send raw message in pieces of wMaxPacketSize bytes.
 */
@@ -201,10 +185,6 @@ int line6_send_raw_message(struct usb_line6 *line6, const char *buffer,
 {
 	int i, done = 0;
 
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-	line6_write_hexdump(line6, 'S', buffer, size);
-#endif
-
 	for (i = 0; i < size; i += line6->max_packet_size) {
 		int partial;
 		const char *frag_buf = buffer + i;
@@ -259,10 +239,6 @@ static int line6_send_raw_message_async_part(struct message *msg,
 			 (char *)msg->buffer + done, bytes,
 			 line6_async_request_sent, msg, line6->interval);
 
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-	line6_write_hexdump(line6, 'S', (char *)msg->buffer + done, bytes);
-#endif
-
 	msg->done += bytes;
 	retval = usb_submit_urb(urb, GFP_ATOMIC);
 
@@ -403,10 +379,6 @@ static void line6_data_received(struct urb *urb)
 	if (urb->status == -ESHUTDOWN)
 		return;
 
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-	line6_dump_urb(urb);
-#endif
-
 	done =
 	    line6_midibuf_write(mb, urb->transfer_buffer, urb->actual_length);
 
@@ -502,10 +474,6 @@ int line6_send_program(struct usb_line6 *line6, u8 value)
 	buffer[0] = LINE6_PROGRAM_CHANGE | LINE6_CHANNEL_HOST;
 	buffer[1] = value;
 
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-	line6_write_hexdump(line6, 'S', buffer, 2);
-#endif
-
 	retval = usb_interrupt_msg(line6->usbdev,
 				   usb_sndintpipe(line6->usbdev,
 						  line6->ep_control_write),
@@ -539,10 +507,6 @@ int line6_transmit_parameter(struct usb_line6 *line6, int param, u8 value)
 	buffer[1] = param;
 	buffer[2] = value;
 
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-	line6_write_hexdump(line6, 'S', buffer, 3);
-#endif
-
 	retval = usb_interrupt_msg(line6->usbdev,
 				   usb_sndintpipe(line6->usbdev,
 						  line6->ep_control_write),
diff --git a/drivers/staging/line6/midi.c b/drivers/staging/line6/midi.c
index 5040729..348d425 100644
--- a/drivers/staging/line6/midi.c
+++ b/drivers/staging/line6/midi.c
@@ -131,9 +131,6 @@ static int send_midi_async(struct usb_line6 *line6, unsigned char *data,
 		dev_err(line6->ifcdev, "Out of memory\n");
 		return -ENOMEM;
 	}
-#ifdef CONFIG_LINE6_USB_DUMP_CTRL
-	line6_write_hexdump(line6, 'S', data, length);
-#endif
 
 	transfer_buffer = kmemdup(data, length, GFP_ATOMIC);
 
-- 
1.8.0