On 06/05/10 07:30, Clemens Ladisch wrote:
dhprince.devel@yahoo wrote:
A specialised HID driver for the CreativeLabs Prodikeys PC-MIDI USB Keyboard. ...
I cannot comment on the input stuff, but the sound stuff looks good overall.
Thanks my first linux coding.
+What: /sys/bus/hid/drivers/prodikeys/.../channel +Date: April 2010 +KernelVersion: 2.6.34 +Contact: Don Prince dhprince.devel@yahoo.co.uk +Descripts/checkpatch.plscription:
Huh?
Fixed. I am being plagued by spurious button presses due to I suspect conflicts between my KVM switch and PS/2 to USB keyboard/Mouse converter.
+What: /sys/bus/hid/drivers/prodikeys/.../sustain +Description:
Allows control (via software) the sustain duration of anote held by the pc-midi driver.Why is this feature needed? Does the device report key releases incorrectly?
The keyboard pretends to support sustain. The driver actually does it.
These three sysfs controls could also be implemented as mixer controls. This would allow them to be automatically saved and restored when the computer is shut down.
- { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS,
USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
Your mailer wrapped long lines and killed the tabs. Please see <Documentation/email-clients.txt>.
And your code looks as if it does not use eight-column tabs for indention.
The code is written using 8 column tabs but you are right thunderbird messed it up. I believe I fixed thunderbird now.
+static void pcmidi_send_note(struct pcmidi_snd *pm, ...
- res = snd_rawmidi_receive(pm->in_substream, buffer, 3);
This return value is never used.
Fixed.
if (!atomic_read(&pms->in_use)) {pms->status = status;pms->note = note;pms->velocity = velocity;atomic_set(&pms->in_use, 1);If you're using the atomic variable to protect against some concurrently executing code, then you have a race condition in the time interval between the read and the set.
Fixed. It was me being overkill, they are actually redundant.
+static void pcmidi_out_tasklet(unsigned long data) ...
- while (1) {
/* just read them and drop them */u8 b;if (snd_rawmidi_transmit(pm->out_substream, &b, 1) != 1) {pm->out_active = 0;break;}This isn't quite how output ports are supposed to be implemented. :-)
I'd guess you want to remove the OUTPUT and DUPLEX flags and to drop all output-related functions.
I know, it was naff, but without it the I get this from "amidi"
$ amidi -l
Dir Device Name
cannot get rawmidi information 2:0: No such file or directory
Is it supposed to print this?
Although the midi still works.
$ amidi -p hw:2,0 --dump
90 2F 5C
80 2F 7F
90 30 52
80 30 7F
90 32 00
80 32 7F
The test code now looks like this
/*dhp snd_component_add(card, "MIDI");*/ err = snd_rawmidi_new(card, card->shortname, 0, 0, 1, &rwmidi); if (err < 0) { pk_error("failed to create pc-midi rawmidi device: error %d\n", err); goto fail; } pm->rwmidi = rwmidi; strncpy(rwmidi->name, card->shortname, sizeof(rwmidi->name)); rwmidi->info_flags = SNDRV_RAWMIDI_INFO_INPUT /*dhp| SNDRV_RAWMIDI_INFO_OUTPUT | SNDRV_RAWMIDI_INFO_DUPLEX*/; rwmidi->private_data = pm;
/*dhp snd_rawmidi_set_ops(rwmidi, SNDRV_RAWMIDI_STREAM_OUTPUT, &pcmidi_out_ops); */ snd_rawmidi_set_ops(rwmidi, SNDRV_RAWMIDI_STREAM_INPUT, &pcmidi_in_ops);
Other command output is printed below.
$ cat /proc/asound/cards
0 [NVidia ]: HDA-Intel - HDA NVidia
HDA NVidia at 0xdfdf8000 irq 21
1 [HDMI ]: HDA-Intel - HDA ATI HDMI
HDA ATI HDMI at 0xdffec000 irq 31
2 [PCMIDI ]: PC-MIDI - PC-MIDI
Prodikeys PC-MIDI Keyboard
$ aconnect -i
client 0: 'System' [type=kernel]
0 'Timer '
1 'Announce '
client 14: 'Midi Through' [type=kernel]
0 'Midi Through Port-0'
client 24: 'PC-MIDI' [type=kernel]
0 'PC-MIDI
$ aconnect -o
client 14: 'Midi Through' [type=kernel]
0 'Midi Through Port-0'
client 128: 'FLUID Synth (Qsynth1)' [type=user]
0 'Synth input port (Qsynth1:0)'
$ aconnect -iol
client 0: 'System' [type=kernel]
0 'Timer '
1 'Announce '
Connecting To: 15:0
client 14: 'Midi Through' [type=kernel]
0 'Midi Through Port-0'
client 24: 'PC-MIDI' [type=kernel]
0 'PC-MIDI '
Connecting To: 128:0
client 128: 'FLUID Synth (Qsynth1)' [type=user]
0 'Synth input port (Qsynth1:0)'
Connected From: 24:0
$ amidi -L
RawMIDI list:
default {
type hw
card {
@func getenv
vars {
0 ALSA_RAWMIDI_CARD
1 ALSA_CARD
}
default {
@func refer
name 'defaults.rawmidi.card'
}
}
device {
@func igetenv
vars {
0 ALSA_RAWMIDI_DEVICE
}
default {
@func refer
name 'defaults.rawmidi.device'
}
}
}
hw {
@args.0 CARD
@args.1 DEV
@args.2 SUBDEV
@args.CARD {
type string
default {
@func getenv
vars {
0 ALSA_RAWMIDI_CARD
1 ALSA_CARD
}
default {
@func refer
name 'defaults.rawmidi.card'
}
}
}
@args.DEV {
type integer
default {
@func igetenv
vars {
0 ALSA_RAWMIDI_DEVICE
}
default {
@func refer
name 'defaults.rawmidi.device'
}
}
}
@args.SUBDEV {
type integer
default -1
}
type hw
card $CARD
device $DEV
subdevice $SUBDEV
hint {
description 'Direct rawmidi driver device'
device $DEV
}
}
virtual {
@args.0 MERGE
@args.MERGE {
type string
default 1
}
type virtual
merge $MERGE
}
+static void pcmidi_in_trigger(struct snd_rawmidi_substream *substream, int up) ...
- if (up)
set_bit(substream->number, &pm->in_triggered);- else
clear_bit(substream->number, &pm->in_triggered);You have only one substream, a boolean variable would suffice.
Fixed.
+int pcmidi_snd_initialise(struct pcmidi_snd *pm) ...
- int out_ports = 1;
- int in_ports = 1;
These variables are not variable and therefore not needed.
Fixed.
- snd_component_add(card, "MIDI");
This function is intended for sound cards that are composed of several components, and the component name is usually a chip name. I think you don't need to call this function at all.
Fixed.
- err = snd_card_register(card);
...
- /* create sysfs variables */
- err = device_create_file(&pm->pk->hdev->dev,
sysfs_device_attr_channel);...
- spin_lock_init(&pm->rawmidi_in_lock);
- init_sustain_timers(pm);
snd_card_register() makes all sound interfaces available to userspace. Initializations for any data that might be accessed from userspace must be done before that call.
Fixed.
Regards, Clemens _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
___________________________________________________________ The all-new Yahoo! Mail goes wherever you go - free your email address from your Internet provider. http://uk.docs.yahoo.com/nowyoucan.html