Re: [PATCH 00/11] ALSA: firewire-motu: add ioctl commands to retrieve information in messages delivered by isoc packet

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Fri, Oct 15, 2021 at 05:54:13PM +0200, Takashi Iwai wrote:
> On Fri, 15 Oct 2021 10:08:15 +0200,
> Takashi Sakamoto wrote:
> > 
> > Hi,
> > 
> > The purpose of this patchset is to add message parser to ALSA
> > firewire-motu driver so that userspace applications can read information
> > in message delivered by isochronous packet as well as PCM frames.
> > The message includes information about hardware meter and user action
> > over hardware component such as knob, and MIDI message bytes.
> > 
> > Models in MOTU FireWire series can be categorized to 4 groups in regard
> > of message mechanism:
> > 
> > Group 1. 828 and 896
> >  * quadlet message to registered destination address
> > Group 2. 828mk2, 896hd, Traveler, 8 pre, Ultralite, 4 pre, and Audio Express
> >  * quadlet message to registered destination address
> >  * message delivered by isochronous packet
> > Group 3. 828mk3, 896mk3, Ultralite mk3, Traveler mk3, and Track 16
> >  * quadlet message to registered destination address
> >  * message delivered by isochronous packet
> >  * block message to registered destination address, including command
> > Group 4. V3HD/V4HD
> >  * quadlet message to registered destination address
> >  * block message to registered destination address
> > 
> > The patchset is for message delivered by isochronous packet in group 2
> > and 3. In Group 2, the message includes information of hardware meter,
> > information of user action over hardware component. The model in Group
> > 2 is called as 'register DSP' in the patchset since parameters of DSP
> > can be configured by asynchronous transaction for register access. In
> > Group 3, the message includes information of hardware meter only. The
> > model in Group 3 is called as 'command DSP' since software and device
> > communicate with commands transferred by asynchronous transaction in
> > regard of DSP parameters. Two types of message parser is going to be
> > added so that the driver caches images for the information. The cache
> > is available for userspace application by ioctl commands newly introduced.
> > 
> > I note that no control element is added for the hardware meters and user
> > actions. It's expected for userspace application to retrieve and parse the
> > information of image then operate for user-defined control element set.
> > 
> > Takashi Sakamoto (11):
> >   ALSA: firewire-motu: add message parser to gather meter information in
> >     register DSP model
> >   ALSA: firewire-motu: add message parser for meter information in
> >     command DSP model
> >   ALSA: firewire-motu: add ioctl command to read cached hardware meter
> >   ALSA: firewire-motu: parse messages for mixer source parameters in
> >     register-DSP model
> >   ALSA: firewire-motu: parse messages for mixer output parameters in
> >     register DSP model
> >   ALSA: firewire-motu: parse messages for output parameters in register
> >     DSP model
> >   ALSA: firewire-motu: parse messages for line input parameters in
> >     register DSP model
> >   ALSA: firewire-motu: parse messages for input parameters in register
> >     DSP model
> >   ALSA: firewire-motu: add ioctl command to read cached DSP parameters
> >   ALSA: firewire-motu: queue event for parameter change in register DSP
> >     model
> >   ALSA: firewire-motu: notify event for parameter change in register DSP
> >     model
> 
> Applied all patches now.  Thanks.

Thanks for your applying the above patches into your tree. I have some
slight concerns about them. I'd like to ask your opinion.

The snd_firewire_motu_command_dsp_meter structure includes array of 32 bit
storage elements. As I added its documentation, the storage includes
IEEE 764 binary32 values between 0.0 and +1.0. In the patchset I use __u32
type since I can find just a few usage of float type in UAPI header. In
driver side, no floating point arithmetic is used since the float value
is just constructed by gathering messages from target device. In the case,
is it adequate to expose the value as float type in UAPI?

Additionally, current ALSA control interface have no support for control
element with float value, like SNDRV_CTL_ELEM_TYPE_IEEE764_BINARY32. As
long as I know, no discussion about it in the list for recent decades.
Have you ever seen such discussion in the list? (Here I intensionally
ignore that we have several binary expressions for floating point since
I'm just interested in the existence of discussion.)


Regards

Takashi Sakamoto