Re: [PATCH] ALSA: dice: add stream format parameters for Weiss devices

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

ei,

On Fri, Jul 28, 2023 at 11:16:11AM +0200, Michele Perrone wrote:
> Hard-coded stream format parameters are added for Weiss Engineering
> FireWire devices. When the device vendor and model match, the parameters
> are copied into the stream format cache. This allows for setting all
> supported sampling rates up to 192kHz, and consequently adjusting the
> number of available I/O channels.
> 
> Signed-off-by: Rolf Anderegg <rolf.anderegg@weiss.ch>
> Signed-off-by: Michele Perrone <michele.perrone@weiss.ch>
> ---
>  sound/firewire/dice/Makefile     |   2 +-
>  sound/firewire/dice/dice-weiss.c | 120 +++++++++++++++++++++++++++++++
>  sound/firewire/dice/dice.c       |  72 +++++++++++++++++++
>  3 files changed, 193 insertions(+), 1 deletion(-)
>  create mode 100644 sound/firewire/dice/dice-weiss.c

Thanks for the patch. I've been waiting so long for such patches to
support Weiss models. I welcome them.

As long as reviewing, I found some format and technical issues on the
patches:

Format issues:
1.unexpected line break
2.tab indentations are replaced with spaces

Technical issues:
3.build error due to missing snd_dice_detect_weiss_formats()
  * prototype of snd_dice_detect_weiss_formats() should be in dice.c
4. category_id in GUID
5.stream formats for INT202

I can correct 1st, 2nd, and 3rd issues. You can find the revised patch
in the top-most of my remote repository[1].

Let me confirm the 4th issue. TCAT defines 'category_id' field in GUID
value of devices. As long as I know, Weiss engineers uses 0x00 in the
field[2]. Is it still correct for the devices supported in the patch?

Next, let us discuss about INT202 stream formats.

+
+/* Weiss INT202: 192kHz unidirectional 2-channel digital Firewire interface
*/
+static const struct dice_weiss_spec int202 = {
+    .tx_pcm_chs = {{2, 2, 2}, {0, 0, 0} },
+    .rx_pcm_chs = {{0, 0, 0}, {0, 0, 0} },
+    .has_midi   = false
+};

(tx/rx should be device-oriented, tx = from device, rx = to device,
please correct)

I assume all of the DICE devices transmit isochronous packets to deliver
presentation time stamp and events (= PCM frames). Then driver software
utilizes the presentation time stamp and the amount of events to construct
payload of isochronous packets into the device.

I program ALSA dice driver based on the assumption, thus ALSA dice driver
doesn't work well without receiving any isochronous packet from the
device. However, the stream formats for INT202 device looks to support
uni-directional operation. Weiss engineers really use DICE chipset like
that? If so, I need to integrate the driver to support the case.


As another topic. I make collection of configuration ROMs[3] to make better
support for the devices in Linux system[4]. I'm pleased if you pick them
up from your devices and dedicate them for the collection.

[1] https://github.com/takaswie/sound/tree/topic/dice/weiss-support
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/sound/firewire/dice/dice.c?h=v6.4#n59
[3] https://github.com/takaswie/am-config-roms/
[4] https://github.com/systemd/systemd/blob/main/hwdb.d/80-ieee1394-unit-function.hwdb


Thanks

Takashi Sakamoto