Re: [Line6linux-devel] POD HD500x working

[Andrej K. <de...@an...>]

Hi there,

On Jun 12, 2017 12:30 AM, "Hans Peter Möller" <hm...@gm...> 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 <de...@an...> wrote:
>
> Hey there,
>
> On Sun, Jun 11, 2017 at 4:07 AM, Hans Peter Möller <hm...@gm...> 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 <de...@an...> wrote:
> >>
> >> Hi Hans Peter,
> >>
> >>
> >> On Sat, Jun 10, 2017 at 5:05 AM, Hans Peter Möller <hm...@gm...>
> >> 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 <de...@an...> 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 <hm...@gm...>
> >> >> 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 <de...@an...> wrote:
> >> >> >>
> >> >> >> Hey,
> >> >> >>
> >> >> >> On Fri, Jun 9, 2017 at 5:35 AM, Hans Peter Möller
> >> >> >> <hm...@gm...>
> >> >> >> 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
> >> >> >
> >> >> >
> >> >
> >> >
> >
> >