Re: [RME MADI/RayDAT/AIO/AES] Changing hdspm_config and hdspm_status

[Takashi Iwai <tiwai@suse.de>]

At Mon, 22 Aug 2011 18:28:59 +0200,
Adrian Knoth wrote:
> 
> Hi!
> 
> Florian Faber has sent me some new bits of his work on hdspm. While the
> changes to hdspm.c are straight-forward, he also cleaned up struct
> hdspm_config and hdspm_status, thus changing the userspace API/ABI:
> 
> 
> diff --git a/include/sound/hdspm.h b/include/sound/hdspm.h
> index 1f59ea2..25eab09 100644
> --- a/include/sound/hdspm.h
> +++ b/include/sound/hdspm.h
> @@ -58,17 +58,20 @@ struct hdspm_peak_rms {
>  /* ------------ CONFIG block IOCTL ---------------------- */
> 
>  struct hdspm_config {
> -	unsigned char pref_sync_ref;
> -	unsigned char wordclock_sync_check;
> -	unsigned char madi_sync_check;
> -	unsigned int system_sample_rate;
> -	unsigned int autosync_sample_rate;
> -	unsigned char system_clock_mode;
> -	unsigned char clock_source;
> -	unsigned char autosync_ref;
> -	unsigned char line_out;
> -	unsigned int passthru;
> -	unsigned int analog_out;
> +	uint32_t dds;       /* dds increment */
> +	uint16_t freq;      /* 32000/44100/48000 */
> +	uint8_t wck_mul;    /* 1/2/4 */
> +	uint8_t clock_mode; /* 0=master, 1=slave */
> +	union {
> +		struct {
> +			uint8_t line_out;     /* 0=disabled, 1=enabled */
> +			uint8_t tms;          /* 0=disabled, 1=enabled */
> +			uint8_t input_select; /* 0=BNC, 1=optical */
> +			uint8_t output_mode;  /* 0=56ch, 1=64ch */
> +			uint8_t output_frame; /* 0=48K, 1=96K */
> +			uint8_t sync_ref;     /* 0=WCK, 1=MADI, 2=TCO/SYNC_IN, 3=SYNC_IN */
> +		} madi;
> +	} card_specific;
>  };
> 
>  #define SNDRV_HDSPM_IOCTL_GET_CONFIG \
> @@ -161,7 +164,6 @@ struct hdspm_status {
>  			uint8_t sync_madi; /* enum hdspm_sync */
>  			uint8_t sync_tco; /* enum hdspm_sync */
>  			uint8_t sync_in; /* enum hdspm_sync */
> -			uint8_t madi_input; /* enum hdspm_madi_input */
>  			uint8_t channel_format; /* enum hdspm_madi_channel_format */
>  			uint8_t frame_format; /* enum hdspm_madi_frame_format */
>  		} madi;
> 
> 
> 
> I guess this means we'd need to redefine two ioctl numbers. Besides
> this, are there any current users of this code?

It's hardly to tell.  But in general, once when the code is
upstreamed, basically you can't change in an incompatible way.
So, changing the existing ioctl struct is no-go.  It's worse than
dropping the ioctl.  Please implement a new ioctl instead.


thanks,

Takashi