Re: [PATCH - [hdspm] RayDAT/AIO 1/2] ALSA: Add support for RME RayDAT and AIO

[Takashi Iwai <tiwai@suse.de>]

At Thu, 27 Jan 2011 11:27:57 +0100,
Adrian Knoth wrote:
> 
> On Thu, Jan 27, 2011 at 09:09:05AM +0100, Jaroslav Kysela wrote:
> 
> >>> +char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
> >>
> >> Err, no, array definitions should be never in a header file.
> >> It doesn't matter whether it's static or not.  The definition should
> >> be in *.c file where it's used.
> 
> We have more than one C file using this information...

Still it doesn't rectify.

> >> So, simply move these array definitions to hdspm.c.
> >
> > I think that the reason is to share these arrays with hdspmixer.
> 
> Exactly. hdspmixer needs this information, too. If we move it to the C
> file, we would need to duplicate the information in hdspmixer. And then
> we forget about it next time hdspm.h needs to be updated.

Then such info isn't appropriate to be static.  If it's dynamic,
this has to be provided via other interface.


> > But I agree, that it's better to move these static values to .c files.
> 
> Ok, I've sent a follow-up patch to avoid to send the whole 180k patch
> for the 4th time.
> 
> I'll now duplicate this information and send a patch against hdspmixer.
> I feel that there should be a better solution like getting the channel
> mapping from the driver (sysfs, proc, ioctl or whatever), but that's for
> some other time.

Exactly.

BTW, now I get the following warnings:
  sound/pci/rme9652/hdspm.c:578:14: warning: ‘texts_sync_status’ defined but not used
  sound/pci/rme9652/hdspm.c:695:13: warning: ‘channel_map_unity_ds’ defined but not used
  sound/pci/rme9652/hdspm.c:706:13: warning: ‘channel_map_unity_qs’ defined but not used
  sound/pci/rme9652/hdspm.c: In function ‘snd_hdspm_hwdep_ioctl’:
  sound/pci/rme9652/hdspm.c:6141:1: warning: the frame size of 2464 bytes is larger than 1024 bytes

Care to fix it?


thanks,

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