Re: [PATCH 1/2] ASoC: cs35l41: CS35L41 Boosted Smart Amplifier

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 31, 2021 at 08:37:30PM -0500, David Rhodes wrote:
> SoC Audio driver for the Cirrus Logic CS35L41 amplifier
> 
> Signed-off-by: David Rhodes <drhodes@xxxxxxxxxxxxxxxxxxxxx>
> ---
> +static void cs35l41_spi_otp_setup(struct cs35l41_private *cs35l41,
> +					bool is_pre_setup, unsigned int *freq)
> +{
> +	struct spi_device *spi = NULL;
> +	u32 orig_spi_freq;
> +
> +	spi = to_spi_device(cs35l41->dev);
> +
> +	if (!spi)
> +		return;
> +
> +	if (is_pre_setup) {
> +		orig_spi_freq = spi->max_speed_hz;
> +		spi->max_speed_hz = CS35L41_SPI_MAX_FREQ_OTP;

We probably need special handling for the case spi->max_speed_hz
is smaller than CS35L41_SPI_MAX_FREQ_OTP here, in that case this
code would increase the SPI speed. If the max speed was set lower
that is probably because something other than the amp in the
system limits it. Track length or the host or something.

> +		spi_setup(spi);
> +		*freq = orig_spi_freq;
> +	} else {
> +		spi->max_speed_hz = *freq;
> +		spi_setup(spi);
> +	}
> +}
...
> +const struct reg_default cs35l41_reg[CS35L41_MAX_CACHE_REG] = {
> +	{CS35L41_TEST_KEY_CTL,			0x00000000},
> +	{CS35L41_USER_KEY_CTL,			0x00000000},
> +	{CS35L41_OTP_CTRL0,			0x00006418},
> +	{CS35L41_OTP_CTRL1,			0x00000000},
> +	{CS35L41_OTP_CTRL3,			0x00000000},
> +	{CS35L41_OTP_CTRL4,			0x00000000},
> +	{CS35L41_OTP_CTRL5,			0x00000030},
> +	{CS35L41_OTP_CTRL6,			0x00000000},
> +	{CS35L41_OTP_CTRL7,			0x00000000},
> +	{CS35L41_OTP_CTRL8,			0x00000000},
> +	{CS35L41_PWR_CTRL1,			0x00000000},
> +	{CS35L41_PWR_CTRL3,			0x01000010},
> +	{CS35L41_CTRL_OVRRIDE,			0x00000002},
> +	{CS35L41_AMP_OUT_MUTE,			0x00000000},
> +	{CS35L41_PROTECT_REL_ERR_IGN,		0x00000000},
> +	{CS35L41_GPIO_PAD_CONTROL,		0x00000000},
> +	{CS35L41_JTAG_CONTROL,			0x00000000},
> +	{CS35L41_PLL_CLK_CTRL,			0x00000010},
> +	{CS35L41_DSP_CLK_CTRL,			0x00000003},
> +	{CS35L41_GLOBAL_CLK_CTRL,		0x00000003},
> +	{CS35L41_DATA_FS_SEL,			0x00000000},
> +	{CS35L41_MDSYNC_EN,			0x00000200},
> +	{CS35L41_MDSYNC_TX_ID,			0x00000000},
> +	{CS35L41_MDSYNC_PWR_CTRL,		0x00000002},
> +	{CS35L41_MDSYNC_DATA_TX,		0x00000000},
> +	{CS35L41_MDSYNC_TX_STATUS,		0x00000002},
> +	{CS35L41_MDSYNC_DATA_RX,		0x00000000},
> +	{CS35L41_MDSYNC_RX_STATUS,		0x00000002},
> +	{CS35L41_MDSYNC_ERR_STATUS,		0x00000000},
> +	{CS35L41_MDSYNC_SYNC_PTE2,		0x00000000},
> +	{CS35L41_MDSYNC_SYNC_PTE3,		0x00000000},
> +	{CS35L41_MDSYNC_SYNC_MSM_STATUS,	0x00000000},
> +	{CS35L41_BSTCVRT_VCTRL1,		0x00000000},
> +	{CS35L41_BSTCVRT_VCTRL2,		0x00000001},
> +	{CS35L41_BSTCVRT_PEAK_CUR,		0x0000004A},
> +	{CS35L41_BSTCVRT_SFT_RAMP,		0x00000003},
> +	{CS35L41_BSTCVRT_COEFF,			0x00002424},
> +	{CS35L41_BSTCVRT_SLOPE_LBST,		0x00007500},
> +	{CS35L41_BSTCVRT_SW_FREQ,		0x01008000},
> +	{CS35L41_BSTCVRT_DCM_CTRL,		0x00002001},

The default needs to be updated to match the register patch here.
regmap_register_patch does a bypassed write so the default should
be set to the patched value.

> +	{CS35L41_BSTCVRT_DCM_MODE_FORCE,	0x00000000},
> +	{CS35L41_BSTCVRT_OVERVOLT_CTRL,		0x00000130},
> +	{CS35L41_VI_VOL_POL,			0x08000800},
> +	{CS35L41_DTEMP_WARN_THLD,		0x00000002},
> +	{CS35L41_DTEMP_EN,			0x00000000},
> +	{CS35L41_VPVBST_FS_SEL,			0x00000001},

ditto.

> +	{CS35L41_SP_ENABLES,			0x00000000},
> +	{CS35L41_SP_RATE_CTRL,			0x00000028},
> +	{CS35L41_SP_FORMAT,			0x18180200},
> +	{CS35L41_SP_HIZ_CTRL,			0x00000002},
> +	{CS35L41_SP_FRAME_TX_SLOT,		0x03020100},
> +	{CS35L41_SP_FRAME_RX_SLOT,		0x00000100},
> +	{CS35L41_SP_TX_WL,			0x00000018},
> +	{CS35L41_SP_RX_WL,			0x00000018},
> +	{CS35L41_DAC_PCM1_SRC,			0x00000008},
> +	{CS35L41_ASP_TX1_SRC,			0x00000018},
> +	{CS35L41_ASP_TX2_SRC,			0x00000019},
> +	{CS35L41_ASP_TX3_SRC,			0x00000020},
> +	{CS35L41_ASP_TX4_SRC,			0x00000021},
> +	{CS35L41_DSP1_RX1_SRC,			0x00000008},
> +	{CS35L41_DSP1_RX2_SRC,			0x00000009},
> +	{CS35L41_DSP1_RX3_SRC,			0x00000018},
> +	{CS35L41_DSP1_RX4_SRC,			0x00000019},
> +	{CS35L41_DSP1_RX5_SRC,			0x00000020},
> +	{CS35L41_DSP1_RX6_SRC,			0x00000021},
> +	{CS35L41_DSP1_RX7_SRC,			0x0000003A},
> +	{CS35L41_DSP1_RX8_SRC,			0x00000001},
> +	{CS35L41_NGATE1_SRC,			0x00000008},
> +	{CS35L41_NGATE2_SRC,			0x00000009},
> +	{CS35L41_AMP_DIG_VOL_CTRL,		0x00008000},
> +	{CS35L41_VPBR_CFG,			0x02AA1905},
> +	{CS35L41_VBBR_CFG,			0x02AA1905},
> +	{CS35L41_VPBR_STATUS,			0x00000000},
> +	{CS35L41_VBBR_STATUS,			0x00000000},
> +	{CS35L41_OVERTEMP_CFG,			0x00000001},
> +	{CS35L41_AMP_ERR_VOL,			0x00000000},
> +	{CS35L41_VOL_STATUS_TO_DSP,		0x00000000},
> +	{CS35L41_CLASSH_CFG,			0x000B0405},
> +	{CS35L41_WKFET_CFG,			0x00000111},
> +	{CS35L41_NG_CFG,			0x00000033},
> +	{CS35L41_AMP_GAIN_CTRL,			0x00000273},
> +	{CS35L41_DAC_MSM_CFG,			0x00580000},
> +	{CS35L41_GPIO1_CTRL1,			0xE1000001},
> +	{CS35L41_GPIO2_CTRL1,			0xE1000001},
> +	{CS35L41_MIXER_NGATE_CFG,		0x00000000},
> +	{CS35L41_MIXER_NGATE_CH1_CFG,		0x00000303},
> +	{CS35L41_MIXER_NGATE_CH2_CFG,		0x00000303},
> +	{CS35L41_CLOCK_DETECT_1,		0x00000000},
> +	{CS35L41_TIMER1_CONTROL,		0x00000000},
> +	{CS35L41_TIMER1_COUNT_PRESET,		0x00000000},
> +	{CS35L41_TIMER1_START_STOP,		0x00000000},
> +	{CS35L41_TIMER1_STATUS,			0x00000000},
> +	{CS35L41_TIMER1_COUNT_READBACK,		0x00000000},
> +	{CS35L41_TIMER1_DSP_CLK_CFG,		0x00000000},
> +	{CS35L41_TIMER1_DSP_CLK_STATUS,		0x00000000},
> +	{CS35L41_TIMER2_CONTROL,		0x00000000},
> +	{CS35L41_TIMER2_COUNT_PRESET,		0x00000000},
> +	{CS35L41_TIMER2_START_STOP,		0x00000000},
> +	{CS35L41_TIMER2_STATUS,			0x00000000},
> +	{CS35L41_TIMER2_COUNT_READBACK,		0x00000000},
> +	{CS35L41_TIMER2_DSP_CLK_CFG,		0x00000000},
> +	{CS35L41_TIMER2_DSP_CLK_STATUS,		0x00000000},
> +	{CS35L41_DFT_JTAG_CONTROL,		0x00000000},
> +	{CS35L41_DIE_STS1,			0x00000000},
> +	{CS35L41_DIE_STS2,			0x00000000},
> +	{CS35L41_TEMP_CAL1,			0x00000000},
> +	{CS35L41_TEMP_CAL2,			0x00000000},
> +};

There are a bunch of registers with status/sts in the name in here with
defaults, normally you wouldn't want a default for a status register,
since it would be a volatile register. Are we sure these are
correct?

> +static const struct snd_kcontrol_new cs35l41_aud_controls[] = {
> +	SOC_SINGLE_SX_TLV("Digital PCM Volume", CS35L41_AMP_DIG_VOL_CTRL,
> +		      3, 0x4CF, 0x391, dig_vol_tlv),
> +	SOC_SINGLE_TLV("AMP PCM Gain", CS35L41_AMP_GAIN_CTRL, 5, 0x14, 0,
> +			amp_gain_tlv),
> +	SOC_SINGLE_RANGE("ASPTX1 Slot Position", CS35L41_SP_FRAME_TX_SLOT, 0,
> +			 0, 7, 0),
> +	SOC_SINGLE_RANGE("ASPTX2 Slot Position", CS35L41_SP_FRAME_TX_SLOT, 8,
> +			 0, 7, 0),
> +	SOC_SINGLE_RANGE("ASPTX3 Slot Position", CS35L41_SP_FRAME_TX_SLOT, 16,
> +			 0, 7, 0),
> +	SOC_SINGLE_RANGE("ASPTX4 Slot Position", CS35L41_SP_FRAME_TX_SLOT, 24,
> +			 0, 7, 0),
> +	SOC_SINGLE_RANGE("ASPRX1 Slot Position", CS35L41_SP_FRAME_RX_SLOT, 0,
> +			 0, 7, 0),
> +	SOC_SINGLE_RANGE("ASPRX2 Slot Position", CS35L41_SP_FRAME_RX_SLOT, 8,
> +			 0, 7, 0),

Probably worth explaining why you want to do this through ALSA
controls rather than set_tdm/set_channel_map.

> +static int cs35l41_component_probe(struct snd_soc_component *component)
> +{
> +	struct cs35l41_private *cs35l41 =
> +		snd_soc_component_get_drvdata(component);
> +
> +	component->regmap = cs35l41->regmap;

Is this necessary I think ASoC handles this internally since the
regmap is already on the CODEC device?

> +
> +	return cs35l41_set_pdata(cs35l41);
> +}

Thanks,
Charles



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux