[alsa-devel] [PATCH] [RFC 3/13] Intel SST driver include headers

[Takashi Iwai]

At Tue, 7 Jul 2009 11:33:23 +0530,
Harsha, Priya wrote:
> 
> >> +/* Firmware Version info */
> >> +struct snd_sst_fw_version {
> >> +	__u8 build;	/* build number*/
> >> +	__u8 minor;	/* minor number*/
> >> +	__u8 major;	/* major number*/
> >> +	__u8 type; /* build type*/
> >> +};
> >> +
> >> +/* Port info structure */
> >> +struct snd_sst_port_info {
> >> +	__u16 port_type;
> >
> >Just wondering -- is there big-endian support?
> We have not tested this on big-endian, Could you tell us that should
> we make it endian safe?

When a field is more than one byte, you'd need endian conversion at
read/write.  I didn't find it thus I suspected it.


> >> +struct snd_sst_vol {
> >> +	unsigned int	stream_id;
> >> +	int		volume;
> >> +	unsigned long	ramp_duration;
> >
> >Are you sure to use long?
> >Long can be different between 32 and 64bit architectures.
> We will change it to u32.
> 
> >
> >> +struct snd_sst_buff_entry {
> >> +	union {
> >> +		void *user;
> >> +		unsigned int offset;
> >> +	} buffer;
> >
> >Is it OK?
> >The pointer and int can be different sizes.
> void* user - is the pointer to the buffer
> unsigned int offset - is the offset inside the buffer area.
> Not sure if I understand your comment. Please let us know if there
> is any issue you see with this structure.

A pointer can be 64bit while offset is always 32bit.
If they don't have to share the same space, why need to be a union?


thanks,

Takashi