Re: [PATCH 00/25 v2] ALSA: support AMDTP variants

From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, clemens@ladisch.de,
	ffado-devel@lists.sf.net
Subject: Re: [PATCH 00/25 v2] ALSA: support AMDTP variants
Date: Sat, 29 Aug 2015 15:00:16 +0900	[thread overview]
Message-ID: <55E14A70.8000903@sakamocchi.jp> (raw)
In-Reply-To: <s5hr3mrmv7m.wl-tiwai@suse.de>

On 2015å¹´08æœˆ25æ—¥ 21:03, Takashi Iwai wrote:
>> For all that, the last part of this patchset is a bot worse. I concide 
>> it, sorry. MIDI functionality of TASCAM FireWire series is enough 
>> complicated to me and I always have a headache at considering about is 
>> (then make some easy mistakes).
>>
>>> Also, I see quite a few code duplications in the series.  For example,
>>> PCM read/write/silence loop is almost identical except for a few lines
>>> of differences inside the loop.  Ditto for MIDI accessors.  Can they
>>> be shared somehow better?
>>
>> I think you mention about the codes in data block processing, such as 
>> read_pcm_s32() in sound/firewire/amdtp-am824.c (originally in amdtp.c). 
>> These functions are called as frequently as 8,000 times per second, thus 
>> it's preferrable to keep the size as small as possible for CPU usage.
>>
>> While, each data block processing includes quite similar codes. In this 
>> meaning, your idea is valid, indeed. But currently, I adhere to the way 
>> to implement different data block processing as what the shape is. 
>> There's no public specification about it and I prefer to keep the codes 
>> as easy to read as possible.
> 
> Well, if you provide simple ops like write_s32(), read_s32(),
> silence_s32(), etc, all codes can be same.  The inner loop calls one
> of this ops, which returns a BE32 value or convert to a native value,
> etc. The performance cost by such a function call isn't too tragic.

I don't like to discuss without actual patches, for this kind of issues.

Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel