Re: [PATCH v3 09/16] ALSA: Oxygen: Modify initialization functions

[Clemens Ladisch <clemens@ladisch.de>]

Roman Volkov wrote:
> Changes in GPIO values and sequence at startup, CS4245
> initialization changed and simplified, according to
> datasheets and reverse-engineering information.
>
>  static void cs4245_init(struct oxygen *chip)
>  {
>  	struct dg *data = chip->model_data;
>
> -	data->cs4245_regs[CS4245_DAC_CTRL_1] =
> -		CS4245_DAC_FM_SINGLE | CS4245_DAC_DIF_LJUST;
> -	data->cs4245_regs[CS4245_ADC_CTRL] =
> -		CS4245_ADC_FM_SINGLE | CS4245_ADC_DIF_LJUST;
> -	data->cs4245_regs[CS4245_SIGNAL_SEL] =
> -		CS4245_A_OUT_SEL_HIZ | CS4245_ASYNCH;
> -	data->cs4245_regs[CS4245_PGA_B_CTRL] = 0;
> -	data->cs4245_regs[CS4245_PGA_A_CTRL] = 0;
> -	data->cs4245_regs[CS4245_ANALOG_IN] =
> -		CS4245_PGA_SOFT | CS4245_PGA_ZERO | CS4245_SEL_INPUT_4;
> -	data->cs4245_regs[CS4245_DAC_A_CTRL] = 0;
> -	data->cs4245_regs[CS4245_DAC_B_CTRL] = 0;
> -	cs4245_registers_init(chip);
> +	/* Save the initial state: codec version, registers */
> +	cs4245_shadow_control(chip, CS4245_SAVE_TO_SHADOW);
> +
> +	data->cs4245_shadow[CS4245_POWER_CTRL] = 0;
> +	data->cs4245_shadow[CS4245_SIGNAL_SEL] = CS4245_A_OUT_SEL_DAC |
> +		CS4245_ASYNCH;
> +	data->cs4245_shadow[CS4245_DAC_CTRL_1] = 0;
> +	data->cs4245_shadow[CS4245_DAC_CTRL_2] = CS4245_DAC_SOFT |
> +		CS4245_DAC_ZERO | CS4245_INVERT_DAC;
> +	data->cs4245_shadow[CS4245_ADC_CTRL] = 0;
> +	data->cs4245_shadow[CS4245_ANALOG_IN] = CS4245_PGA_SOFT |
> +		CS4245_PGA_ZERO;

Please don't remove symbols like "CS4245_DAC_FM_SINGLE | CS4245_DAC_DIF_LJUST";
this would imply that you don't care about these settings.

Why don't you initialize registers like PGA/DAC_A/B?

> +void dg_init(struct oxygen *chip)
>  {
> -	data->output_sel = 0;
> -	data->input_sel = 3;
> -	data->hp_vol_att = 2 * 16;

These initializations should not be removed until all the other code
that uses them is also removed.

> -	oxygen_clear_bits16(chip, OXYGEN_GPIO_DATA,
> -			    GPIO_INPUT_ROUTE | GPIO_HP_REAR);
> +	oxygen_write16(chip, OXYGEN_GPIO_DATA, GPIO_HP_REAR | GPIO_INPUT_ROUTE);

Why are these bits now initialized to the opposite value?


Regards,
Clemens