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