Re: Controlling the Tascam FW-1884

[Scott Bahling <sbahling@suse.com>]

Hi Takashi,

On Sun, 2018-10-14 at 21:09 +0200, Scott Bahling wrote:
> On Sat, 2018-10-13 at 19:40 +0900, Takashi Sakamoto wrote:
> 
> > For FW-1082 these quadlets includes value of movable fader as well,
> > however unlike FW-1884 they have fluctuate quirk. So I think it
> > reasonable for us to program this module to ignore them.
> 
> Ah, understood.
> 
> > Instead, let us program applications so that they call
> > 'hinawa_snd_tscm_get_status()' periodically to get current value of
> > these faders between touch and untouch event on quadlet 05? This
> > take applications to consume CPU time more efficiently than handling
> > many events.
> 
> That should work.

I have tried this, but the endianness of the status bits passed via this
method seems to be wrong.

I noticed in your latest kernel code[1], that you don't convert the firewire
data to local CPU endian when storing in the "after" variable which ends up
getting pushed into the status structure as big endian:


    after = buffer[s->data_block_quadlets - 1];
...
...
...
    tscm->status[index] = after;


Shouldn't that be:

    after = be32_to_cpu(buffer[s->data_block_quadlets - 1]);

which also would remove the need for later conversions in that block of
code?

I have a tested patch which I will send as a pull request once github is
functioning again.

> ...
> > Well, as a result, ALSA firewire-tascam driver handles 'edge-trigge'
> > events except for jog wheel and knobs. For this kind of event, it's
> > useful to notify before/after value when emitting notification. So I'd
> > like to change structure passed to UAPI so that:
> > 
> >   struct snd_firewire_tascam_control {
> >          unsigned int index;
> > -       __u32 flags;
> > +       __u32 before;
> > +       __u32 after;
> >   };
> > 
> > Would I request your opinion? You can see patches here:
> >   - 
> > https://github.com/takaswie/snd-firewire-improve/tree/topic/tascam-userspace-take3
> >   - https://github.com/takaswie/libhinawa/tree/topic/tascam-userspace-take3
> > 
> > You can receive 'control' event of 'HinawaSndTscm' GObject with these
> > two values:
> > 
> 
> ...
> 
> Sounds reasonable. For state change detection, I currently store the
> "before" state of each control in my application. Your new driver
> implementation will mean not needing to maintain my own state
> structures for that purpose.
> 
> I will try it out in the next few days.

I have only done a small amount of testing, but so far the control event
setup is working fine.

-Scott


[1] https://github.com/takaswie/snd-firewire-improve/blob/topic/tascam-userspace-take3/sound/firewire/tascam/amdtp-tascam.c#L132