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