On Fri, 20 Nov 2015 03:28:39 +0100, Takashi Sakamoto wrote:
Hi,
On Nov 18 2015 23:17, Takashi Iwai wrote:
On Sun, 15 Nov 2015 10:26:00 +0100, Takashi Sakamoto wrote:
- err = spkr_volume_command(oxfw, &oxfw->volume_min,
- if (strcmp(oxfw->card->driver, "FireWave") == 0) {
spkr->mixer_channels = 6;spkr->mute_fb_id = 0x01;spkr->volume_fb_id = 0x02;- }
- if (strcmp(oxfw->card->driver, "FWSpeakers") == 0) {
spkr->mixer_channels = 1;spkr->mute_fb_id = 0x01;spkr->volume_fb_id = 0x01;- }
What's the merit of such explicit individual conditional over the constant table in the current implementation? The latter is more error-prone and simpler in general.
I'm also concerned about it. Yes, the usage of 'struct ieee1394_device_id.driver_data' is nicer than thse condition statements, in this point.
My intension of a part of this patch series is to enclose model-dependent parameters inner model-dependent files, instead of adding module-public structure. This idea, itself, is not so bad, I think.
But it's not worth to do open-code in each place, either.
If there were hundreds of different parameters per model, it would be good to hide somehow locally. But in this case, it's only two fields (and even optional), so no big matter, IMO.
Takashi
There may be better ways to detect models and assign to structure but I don't still find it.
Thanks
Takashi Sakamoto