On Mon, Jun 12, 2017 at 4:33 PM, Hans Peter Möller <[email protected]> wrote: > What do you mean with "release of workqueue" ?? The podhd_disconnect > function? > > The other two (claim interface in and sysfs out) I think it might by solved > with creating a new if in podhd_init funtion: > > > static int podhd_init(struct usb_line6 *line6, > const struct usb_device_id *id) > { > int err; > struct usb_line6_podhd *pod = (struct usb_line6_podhd *) line6; > struct usb_interface *intf; > > line6->disconnect = podhd_disconnect; > > if (pod->line6.properties->capabilities & LINE6_CAP_CONTROL) { > /* claim the data interface */ > intf = usb_ifnum_to_if(line6->usbdev, > pod->line6.properties->ctrl_if); > if (!intf) { > dev_err(pod->line6.ifcdev, "interface %d not found\n", > pod->line6.properties->ctrl_if); > return -ENODEV; > } > > err = usb_driver_claim_interface(&podhd_driver, intf, NULL); > if (err != 0) { > dev_err(pod->line6.ifcdev, "can't claim interface %d, error > %d\n", > pod->line6.properties->ctrl_if, err); > return err; > } > + } > > + if (pod->line6.properties->capabilities & LINE6_CAP_CONTROL_INIT) { > /* create sysfs entries: */ > err = snd_card_add_dev_attr(line6->card, &podhd_dev_attr_group); > if (err < 0) > return err; > } > > if (pod->line6.properties->capabilities & LINE6_CAP_PCM) { > /* initialize PCM subsystem: */ > err = line6_init_pcm(line6, > (id->driver_info == LINE6_PODX3 || > id->driver_info == LINE6_PODX3LIVE) ? &podx3_pcm_properties : > &podhd_pcm_properties); > if (err < 0) > return err; > } > > - if (!(pod->line6.properties->capabilities & LINE6_CAP_CONTROL)) { > + if (!(pod->line6.properties->capabilities & LINE6_CAP_CONTROL_INIT)) { > /* register USB audio system directly */ > return podhd_startup_finalize(pod); > } > > /* init device and delay registering */ > init_timer(&pod->startup_timer); > INIT_WORK(&pod->startup_work, podhd_startup_workqueue); > podhd_startup(pod); > return 0; > } > > > That's what you where thinking? I haven't tested it, I will do as soon as I > can. >
Yup, exactly this (including the disconnect function). :-) A. > On Mon, Jun 12, 2017 at 1:46 AM, Andrej Kruták <[email protected]> wrote: >> >> Hi there, >> >> On Jun 12, 2017 12:30 AM, "Hans Peter Möller" <[email protected]> wrote: >> >> You were right, I forgot to test if disabling the initialization >> worked. I tested and it worked, the "receive length failed (error >> -11)" error disapear. >> I actually change all of the LINE6_CAP_CONTROL to >> LINE6_CAP_CONTROL_INIT and it worked. I don't know in 4.12 but in 4.10 >> there is used in >> >> static void podhd_disconnect(struct usb_line6 *line6) >> { >> struct usb_line6_podhd *pod = (struct usb_line6_podhd *)line6; >> >> /* if (pod->line6.properties->capabilities & LINE6_CAP_CONTROL) {*/ >> if (pod->line6.properties->capabilities & LINE6_CAP_CONTROL_INIT) { >> struct usb_interface *intf; >> >> del_timer_sync(&pod->startup_timer); >> cancel_work_sync(&pod->startup_work); >> >> intf = usb_ifnum_to_if(line6->usbdev, >> pod->line6.properties->ctrl_if); >> usb_driver_release_interface(&podhd_driver, intf); >> } >> } >> >> and in podhd_init is also this: >> >> if (pod->line6.properties->capabilities & LINE6_CAP_CONTROL_INIT) { >> /* claim the data interface */ >> intf = usb_ifnum_to_if(line6->usbdev, >> pod->line6.properties->ctrl_if); >> if (!intf) { >> dev_err(pod->line6.ifcdev, "interface %d not found\n", >> pod->line6.properties->ctrl_if); >> return -ENODEV; >> } >> >> err = usb_driver_claim_interface(&podhd_driver, intf, NULL); >> if (err != 0) { >> dev_err(pod->line6.ifcdev, "can't claim interface %d, error >> %d\n", >> pod->line6.properties->ctrl_if, err); >> return err; >> } >> >> /* create sysfs entries: */ >> err = snd_card_add_dev_attr(line6->card, &podhd_dev_attr_group); >> if (err < 0) >> return err; >> } >> >> It's better to change both of the to LINE6_CAP_CONTROL_INIT? >> >> I found out that the 500x need the LINE6_CAP_CONTROL in driver.c, in >> the probe (maybe also in suspend and resume). >> >> >> >> Hmm, looking at the code, I think we'll have to refactor it a bit >> more. If you want to do it - the usb_claim/release needs to be done >> always for CAP_CONTROL. For !CONTROL_INIT, we should skip the init and >> release of workqueue and sysfs stuff. I can create the patch too and >> send it to you for testing, if you want. Don't worry, you won't lose >> credits :-) >> >> >> >> MIDI: >> I added the LINE6_CAP_CONTROL_MIDI to my pod and also added the >> midi_init procedure in podhd.c, aplaymidi -l sees the midi ports of my >> pod, but my system freezes (apparenly when my pod sends data), so >> there's more to do here. >> >> Could I do midi via the quirck? Or one device should only has one driver? >> >> >> As I wrote, the hw interface likely doesn't send midi since +/- pod >> x3. It's some proprietary binary protocol instead, so getting it to >> work is not trivial. At very least, you'd have to write a translation >> layer, but I reckon such task would be better suited for a userspace >> app. >> >> >> >> Anyway, I will wrap-up to submit the patch with audio only. I will >> read teh link you send me on how to do it. >> >> When you send me the info of how to do it, you meant this? >> https://www.kernel.org/doc/html/v4.11/process/submitting-patches.html >> because I search there for line6 and didn't found anything. >> >> >> Yup.. >> >> The generic alsa-devel mailing list is ok (see MAINTAINERS file, SOUND >> section), just check the recent commit log of the driver and put a few >> relevant people on cc. You can send me the patch first if you want, >> for a quick pre-review (I'd just check that we understood each other >> and you didn't break POD X3 :) ). >> >> Regarding POD HD X, it probably depends on how sure you are that it'll >> work. If lsusb is the same, it's probably safe to assume it and add. >> If you don't have even lsusb, I'd go the safe way for now. >> >> In any case, split the patch into at least two (add CONTROL_INIT & >> pod500x(+hdx) support). >> >> >> -- >> Greetings, >> >> Andrej >> >> >> On Sun, Jun 11, 2017 at 2:42 AM, Andrej Kruták <[email protected]> wrote: >> > >> > Hey there, >> > >> > On Sun, Jun 11, 2017 at 4:07 AM, Hans Peter Möller <[email protected]> >> > wrote: >> > > Hi Andrej, >> > > I believe I will have to sniff to get rid of that error. >> > > >> > >> > Well, if you insist... :-) I guess since it works without the >> > initialization, and noone really needs the device serial number, you >> > could for now just disable the code. But of course sniffing is more >> > educational and systematic... >> > >> > >> > > Is there a reason why in kernel 4.10 the podhd.c has no midi init? Now >> > > I >> > > don't see the pod midi ports, I remember that in some of my other >> > > tries >> > > (kernel 4.4 or 4.8 or quirck) I could see the midi ports of my pod in >> > > qjacktl connections (but I never test them). >> > > How is it in Kernel 4.12? How they init midi? (I tried to find the >> > > tree in >> > > inet but I couldn't get it). Maybe they drop midi at all. >> > > >> > >> > Perhaps you did it with the quirk thing. But the podhd driver, AFAIK, >> > never supported MIDI. Maybe pod hdNNN devices support it, but t least >> > POD X3 doesn't have a native midi interface (compared to previous >> > models) --> you would have to emulate it via the bulk binary protocol >> > somehow. Check LINE6_CAP_CONTROL_MIDI in driver.c >> > >> > >> > > In dmesg I see this warning after connecting, could this mean >> > > something? >> > > >> > > [ 530.888811] WARNING: CPU: 1 PID: 2889 at >> > > >> > > /build/linux-hwe-edge-vebCva/linux-hwe-edge-4.10.0/drivers/usb/core/hcd.c:1587 >> > > usb_hcd_map_urb_for_dma+0x382/0x560 >> > > [ 530.888813] transfer buffer not dma capable >> > > >> > >> > This sounds like host USB controller issue, nothing directly related >> > to the podhd driver probably. >> > >> > >> > -- >> > Greetings, >> > >> > Andrej >> > >> > >> > > >> > > On Sat, Jun 10, 2017 at 4:48 AM, Andrej Kruták <[email protected]> wrote: >> > >> >> > >> Hi Hans Peter, >> > >> >> > >> >> > >> On Sat, Jun 10, 2017 at 5:05 AM, Hans Peter Möller >> > >> <[email protected]> >> > >> wrote: >> > >> >> > >> > I made the tests, >> > >> > - Removing LINE6_CAP_CONTROL disable playback in the unit, I can >> > >> > still >> > >> > capture but no sound come out of the unit. The "receive length >> > >> > failed >> > >> > (error >> > >> > -11)" disappear. Since Initialization is required for playback, the >> > >> > .ctrl_if >> > >> > is needed and thus the simple patch will only work in Kernel 4.9 >> > >> > and >> > >> > above. >> > >> >> > >> I'm not sure it would be accepted into the maintenance branches >> > >> (<4.13) anyway, so that shouldn't matter. In the end .ctrl_if is not >> > >> needed for the <4.12 kernels anyhow (it's basically there just to >> > >> lock >> > >> the USB device access for the kernel). >> > >> >> > >> >> > >> > -Removing LINE6_CAP_IN_NEEDS_OUT has no effect, so it's not needed. >> > >> > >> > >> > We still have the "receive length failed (error -11)" error, any >> > >> > other >> > >> > ideas >> > >> > of how to solve it? Or is not necessary? >> > >> > >> > >> >> > >> Probably the init protocol is different then (no way to tell unless >> > >> you sniff some stuff from windows). You can check one last thing - >> > >> whether line6_read_serial_number() returns a reasonable value. Most >> > >> likely not, but... >> > >> >> > >> I would say the best solution will be to add another flag, something >> > >> like LINE6_CAP_CONTROL_INIT - and adjust podhd_init() like: >> > >> >> > >> - if (!(pod->line6.properties->capabilities & LINE6_CAP_CONTROL)) >> > >> { >> > >> + if (!(pod->line6.properties->capabilities & >> > >> LINE6_CAP_CONTROL_INIT)) >> > >> { >> > >> /* register USB audio system directly */ >> > >> return podhd_startup_finalize(pod); >> > >> } >> > >> >> > >> The flag would be only set for POD X3 for now... >> > >> >> > >> >> > >> > Just of curiosity, is there any difference in the usb_device_id >> > >> > podhd_id_table[] if I use >> > >> > { LINE6_DEVICE(0x4159), .driver_info = LINE6_PODHD500X }, >> > >> > instead of: >> > >> > { LINE6_IF_NUM(0x4159, 0), .driver_info = LINE6_PODHD500X }, >> > >> > >> > >> > The current table is: >> > >> > /* table of devices that work with this driver */ >> > >> > static const struct usb_device_id podhd_id_table[] = { >> > >> > /* TODO: no need to alloc data interfaces when only audio is >> > >> > used */ >> > >> > { LINE6_DEVICE(0x5057), .driver_info = LINE6_PODHD300 }, >> > >> > { LINE6_DEVICE(0x5058), .driver_info = LINE6_PODHD400 }, >> > >> > { LINE6_IF_NUM(0x414D, 0), .driver_info = LINE6_PODHD500_0 }, >> > >> > { LINE6_IF_NUM(0x414D, 1), .driver_info = LINE6_PODHD500_1 }, >> > >> > { LINE6_IF_NUM(0x414A, 0), .driver_info = LINE6_PODX3 }, >> > >> > { LINE6_IF_NUM(0x414B, 0), .driver_info = LINE6_PODX3LIVE }, >> > >> > { LINE6_IF_NUM(0x4159, 0), .driver_info = LINE6_PODHD500X }, >> > >> > {} >> > >> > }; >> > >> > >> > >> >> > >> This depends on how the device looks like. If there is only one USB >> > >> interface, LINE6_DEVICE should be sufficient. However, POD X3 has one >> > >> interface for audio (isochronous endpoints) and one for bulk >> > >> transfers, so it needs LINE6_IF_NUM() to match the right >> > >> device=interface (plus .ctrl_if for locking of the other one...). >> > >> >> > >> -- >> > >> Best regards, >> > >> >> > >> Andrej >> > >> >> > >> >> > >> > On Fri, Jun 9, 2017 at 11:50 AM, Andrej Kruták <[email protected]> >> > >> > wrote: >> > >> >> >> > >> >> Ah yes, that one patch... I already forgot about it (and of course >> > >> >> didn't check the sources at the time of merge in 4.12 :)) Ok, but >> > >> >> you >> > >> >> shouldn't need the ctrl_if thing if you only use the audio stuff. >> > >> >> >> > >> >> Regarding the maxpacket error, my prints the same - Line6 >> > >> >> apparently >> > >> >> violates the USB specification (AFAIR bulk endpoints on HS USB 64 >> > >> >> is >> > >> >> invalid maxsize (aka only valid for FullSpeed USB)...). In any >> > >> >> case, >> > >> >> nothing to worry about... >> > >> >> >> > >> >> >> > >> >> -- >> > >> >> Greetings, >> > >> >> >> > >> >> Andrej >> > >> >> >> > >> >> >> > >> >> On Fri, Jun 9, 2017 at 4:42 PM, Hans Peter Möller >> > >> >> <[email protected]> >> > >> >> wrote: >> > >> >> > Hi, >> > >> >> > I will thest droping those things and see what happened. >> > >> >> > What about the " config 1 interface 1 altsetting 0 bulk endpoint >> > >> >> > 0x1 >> > >> >> > has >> > >> >> > invalid maxpacket 64" ??? It has appear since the first time I >> > >> >> > plug >> > >> >> > my >> > >> >> > pod >> > >> >> > with the stock kernel 4.4. >> > >> >> > >> > >> >> > in your kernel 4.12 the if_ctrl doesn't appear? How did they >> > >> >> > implemented >> > >> >> > the >> > >> >> > x3 then? >> > >> >> > >> > >> >> > here is the podhd.c that come in my kernel >> > >> >> > >> > >> >> > >> > >> >> > >> > >> >> > https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/xenial/tree/sound/usb/line6/podhd.c?h=hwe-edge >> > >> >> > and this is the one from torvalds >> > >> >> > >> > >> >> > https://github.com/torvalds/linux/blob/master/sound/usb/line6/podhd.c >> > >> >> > >> > >> >> > both have the x3 implementation with the ctr_if (and mention >> > >> >> > you). >> > >> >> > >> > >> >> > >> > >> >> > >> > >> >> > On Fri, Jun 9, 2017 at 1:51 AM, Andrej Kruták <[email protected]> >> > >> >> > wrote: >> > >> >> >> >> > >> >> >> Hey, >> > >> >> >> >> > >> >> >> On Fri, Jun 9, 2017 at 5:35 AM, Hans Peter Möller >> > >> >> >> <[email protected]> >> > >> >> >> wrote: >> > >> >> >> > Hi Andrej, >> > >> >> >> > good news!! >> > >> >> >> > I can capture and listen from my pod. >> > >> >> >> > >> > >> >> >> >> > >> >> >> Perfect! >> > >> >> >> >> > >> >> >> >> > >> >> >> > I compare the lsusb from de x3 and I found that it has the >> > >> >> >> > same >> > >> >> >> > alternative >> > >> >> >> > setting issue than the hd500x, which is different from hd300 >> > >> >> >> > (I >> > >> >> >> > couldn't >> > >> >> >> > get >> > >> >> >> > one from h400 nor from h500). In x3 and 500x the altsetting >> > >> >> >> > is >> > >> >> >> > different >> > >> >> >> > for >> > >> >> >> > audio than for data. >> > >> >> >> > Since X3 is included in kernel 4.9, I decided to go over >> > >> >> >> > kernel >> > >> >> >> > 4.9 >> > >> >> >> > and >> > >> >> >> > use >> > >> >> >> > that code which also appear to be more updated than the one >> > >> >> >> > here. >> > >> >> >> > >> > >> >> >> > In podhd.c I copy the same configuration used for X3. The >> > >> >> >> > .ctr_if=1 >> > >> >> >> > is >> > >> >> >> > the >> > >> >> >> > difference from the others and it make the difference. This >> > >> >> >> > are >> > >> >> >> > the >> > >> >> >> > modifications I made: >> > >> >> >> > >> > >> >> >> > enum { >> > >> >> >> > LINE6_PODHD300, >> > >> >> >> > LINE6_PODHD400, >> > >> >> >> > LINE6_PODHD500_0, >> > >> >> >> > LINE6_PODHD500_1, >> > >> >> >> > LINE6_PODX3, >> > >> >> >> > LINE6_PODX3LIVE, >> > >> >> >> > LINE6_PODHD500X >> > >> >> >> > }; >> > >> >> >> > >> > >> >> >> > /* table of devices that work with this driver */ >> > >> >> >> > static const struct usb_device_id podhd_id_table[] = { >> > >> >> >> > /* TODO: no need to alloc data interfaces when only audio >> > >> >> >> > is >> > >> >> >> > used >> > >> >> >> > */ >> > >> >> >> > { LINE6_DEVICE(0x5057), .driver_info = LINE6_PODHD300 >> > >> >> >> > }, >> > >> >> >> > { LINE6_DEVICE(0x5058), .driver_info = LINE6_PODHD400 >> > >> >> >> > }, >> > >> >> >> > { LINE6_IF_NUM(0x414D, 0), .driver_info = >> > >> >> >> > LINE6_PODHD500_0 }, >> > >> >> >> > { LINE6_IF_NUM(0x414D, 1), .driver_info = >> > >> >> >> > LINE6_PODHD500_1 }, >> > >> >> >> > { LINE6_IF_NUM(0x414A, 0), .driver_info = LINE6_PODX3 }, >> > >> >> >> > { LINE6_IF_NUM(0x414B, 0), .driver_info = LINE6_PODX3LIVE >> > >> >> >> > }, >> > >> >> >> > { LINE6_IF_NUM(0x4159, 0), .driver_info = LINE6_PODHD500X >> > >> >> >> > }, >> > >> >> >> > {} >> > >> >> >> > }; >> > >> >> >> > >> > >> >> >> > >> > >> >> >> > >> > >> >> >> > >> > >> >> >> > [LINE6_PODHD500X] = { >> > >> >> >> > .id = "PODHD500X", >> > >> >> >> > .name = "POD HD500x", >> > >> >> >> > .capabilities = LINE6_CAP_CONTROL >> > >> >> >> > | LINE6_CAP_PCM | LINE6_CAP_HWMON | >> > >> >> >> > LINE6_CAP_IN_NEEDS_OUT, >> > >> >> >> > .altsetting = 1, >> > >> >> >> > .ep_ctrl_r = 0x81, >> > >> >> >> > .ep_ctrl_w = 0x01, >> > >> >> >> > .ctrl_if = 1, >> > >> >> >> > .ep_audio_r = 0x86, >> > >> >> >> > .ep_audio_w = 0x02, >> > >> >> >> > }, >> > >> >> >> > >> > >> >> >> > >> > >> >> >> > I still get some errors/warning in dmesg, the maxpacket are >> > >> >> >> > the >> > >> >> >> > data >> > >> >> >> > ep >> > >> >> >> > so >> > >> >> >> > that is data. The "receive length failed (error -11)" don't >> > >> >> >> > know >> > >> >> >> > what >> > >> >> >> > it >> > >> >> >> > is, >> > >> >> >> > maybe initialization? Because it only appear this two times. >> > >> >> >> > >> > >> >> >> > Here is dmesg output: >> > >> >> >> > >> > >> >> >> > [ 4001.944114] usb 1-3: new high-speed USB device number 11 >> > >> >> >> > using >> > >> >> >> > ehci-pci >> > >> >> >> > [ 4002.063668] usb 1-3: config 1 interface 1 altsetting 0 >> > >> >> >> > bulk >> > >> >> >> > endpoint >> > >> >> >> > 0x1 >> > >> >> >> > has invalid maxpacket 64 >> > >> >> >> > [ 4002.063677] usb 1-3: config 1 interface 1 altsetting 0 >> > >> >> >> > bulk >> > >> >> >> > endpoint >> > >> >> >> > 0x81 >> > >> >> >> > has invalid maxpacket 64 >> > >> >> >> > [ 4002.064779] usb 1-3: New USB device found, idVendor=0e41, >> > >> >> >> > idProduct=4159 >> > >> >> >> > [ 4002.064785] usb 1-3: New USB device strings: Mfr=1, >> > >> >> >> > Product=2, >> > >> >> >> > SerialNumber=0 >> > >> >> >> > [ 4002.064790] usb 1-3: Product: POD HD500X >> > >> >> >> > [ 4002.064794] usb 1-3: Manufacturer: Line 6 >> > >> >> >> > [ 4002.067896] snd_usb_podhd 1-3:1.0: Line 6 POD HD500x found >> > >> >> >> > [ 4002.069846] snd_usb_podhd 1-3:1.0: Line 6 POD HD500x now >> > >> >> >> > attached >> > >> >> >> > [ 4002.577334] snd_usb_podhd 1-3:1.0: receive length failed >> > >> >> >> > (error >> > >> >> >> > -11) >> > >> >> >> > [ 4002.579660] snd_usb_podhd 1-3:1.0: receive length failed >> > >> >> >> > (error >> > >> >> >> > -11) >> > >> >> >> > >> > >> >> >> >> > >> >> >> I think this is caused by the device having different initial >> > >> >> >> setup >> > >> >> >> requirements. But it also (most likely) means that it doesn't >> > >> >> >> need >> > >> >> >> any >> > >> >> >> setup. Thus drop the LIBE6_CAP_CONTROL thing, see if it still >> > >> >> >> works. >> > >> >> >> >> > >> >> >> In addition, can you try also dropping LINE6_CAP_IN_NEEDS_OUT - >> > >> >> >> and >> > >> >> >> check whether the stand-alone recording works? >> > >> >> >> >> > >> >> >> Lastly, I don't see any ctrl_if in the structure? Not in 4.0 >> > >> >> >> ... >> > >> >> >> 4.12-rc kernels at least... >> > >> >> >> >> > >> >> >> >> > >> >> >> > I believe this should be added to the podhd.c delivered in >> > >> >> >> > the >> > >> >> >> > kernel >> > >> >> >> > but I >> > >> >> >> > dont know how to do that. >> > >> >> >> > >> > >> >> >> >> > >> >> >> Check linux/Documentation/process/submitting-patches.rst. In >> > >> >> >> short - >> > >> >> >> just get inspired in the commit history of sound/usb/line6, >> > >> >> >> create a >> > >> >> >> similar patch (git format-patch) and send it to alsa-devel >> > >> >> >> mailinglist. >> > >> >> >> >> > >> >> >> Alternatively, once you test the above, I can submit the patch >> > >> >> >> on >> > >> >> >> your >> > >> >> >> behalf too. >> > >> >> >> >> > >> >> >> >> > >> >> >> -- >> > >> >> >> Best regards, >> > >> >> >> >> > >> >> >> Andrej >> > >> >> > >> > >> >> > >> > >> > >> > >> > >> > > >> > > > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Line6linux-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/line6linux-devel
