[alsa-devel] [PATCH 4/9] ALSA: oxfw: apply model-specific functionality framework to firewire-speakers

[Takashi Iwai]

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
>