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

[Takashi Iwai]

At Fri,  3 Jul 2009 12:32:25 +0530,
Vinod Koul wrote:
> 
> This patch adds the intel_sst.h header file that is places in the
> include folder for MAD driver (ALSA sound card driver for the platform
> given in later patches) to use. This file contains the definitions
> of interfaces exposed by the SST drivers along with the definitions of
> all the controls for the sound card to be used by MAD driver.
> intel_sst_ioctl.h - this file exposes the ioctls definitions
> for player/middleware to use. Many of the definitions are reuse from ALSA
> 
> Signed-off-by: Vinod Koul <vinod.koul at intel.com>
> Signed-off-by: Harsha Priya <priya.harsha at intel.com>
> 
> 	new file:   include/sound/intel_sst.h
> 	new file:   include/sound/intel_sst_ioctl.h
> ---
>  include/sound/intel_sst.h       |  259 +++++++++++++++++++++++++++++
>  include/sound/intel_sst_ioctl.h |  351 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 610 insertions(+), 0 deletions(-)
>  create mode 100644 include/sound/intel_sst.h
>  create mode 100644 include/sound/intel_sst_ioctl.h
> 
> diff --git a/include/sound/intel_sst.h b/include/sound/intel_sst.h
> new file mode 100644
> index 0000000..c709390
> --- /dev/null
> +++ b/include/sound/intel_sst.h
(snip)
> +static inline int sst_sc_reg_access(struct sc_reg_access *sc_access,
> +							int type, int num_val)
> +{
> +#ifndef CONFIG_SST_IPC_NOT_INCLUDED
> +	int i, retval = 0, j = 0, k = 0, count = 0;
> +	struct ipc_pmic_reg_data reg_data;
> +	struct ipc_pmic_mod_reg_data pmic_mod_reg = {0};

This is way too big as an inline function.

> diff --git a/include/sound/intel_sst_ioctl.h b/include/sound/intel_sst_ioctl.h
> new file mode 100644
> index 0000000..848ea26
> --- /dev/null
> +++ b/include/sound/intel_sst_ioctl.h
> +
> +enum sst_codec_types {
> +/*  AUDIO/MUSIC	CODEC Type Definitions */
> +	SST_CODEC_TYPE_UNKNOWN = 0,
> +	SST_CODEC_TYPE_PCM,	/* Pass through Audio codec */
> +	SST_CODEC_TYPE_MP3,
> +	SST_CODEC_TYPE_MP24,
> +	SST_CODEC_TYPE_AAC,
> +	SST_CODEC_TYPE_AACP,
> +	SST_CODEC_TYPE_eAACP,
> +	SST_CODEC_TYPE_WMA9,
> +	SST_CODEC_TYPE_WMA10,
> +	SST_CODEC_TYPE_WMA10P,
> +	SST_CODEC_TYPE_RA,
> +	SST_CODEC_TYPE_DDAC3,
> +	SST_CODEC_TYPE_STEREO_TRUE_HD,
> +	SST_CODEC_TYPE_STEREO_HD_PLUS,
> +
> +	/*  VOICE CODEC Type Definitions */
> +	SST_CODEC_TYPE_VOICE_PCM = 0x21, /* Pass through voice codec */
> +	SRC = 0x64,
> +	MIXER = 0x65,
> +	DOWN_MIXER = 0x66,
> +	VOLUME_CONTROL = 0x67,
> +	OEM1 = 0xC8,
> +	OEM2 = 0xC9,

These terms are too generic to be used as public definitions.
Better to put a prefix.

> +/* 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?

> +/* Target selection per device structure */
> +struct snd_sst_slot_info {
> +	__u8 mix_enable;		/* Mixer enable or disable */
> +	__u8 device_type;
> +	__u8 device_instance; 	/* 0, 1, 2 */
> +	__u16 slot[2];
> +	struct snd_sst_pmic_config pcm_params;
> +	enum snd_sst_target_type type;

Better to use another type with a strict size instead of enum.

> +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.

> +struct snd_sst_buff_entry {
> +	union {
> +		void *user;
> +		unsigned int offset;
> +	} buffer;

Is it OK?
The pointer and int can be different sizes.


Takashi