On Tue, Jan 14, 2020 at 10:56:28AM +0800, jack.yu@realtek.com wrote:
+++ b/sound/soc/codecs/rt1015.c @@ -0,0 +1,1016 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- rt1015.c -- RT1015 ALSA SoC audio amplifier driver
Please make the entire comment a C++ so things look more consistent.
+static const struct reg_sequence init_list[] = {
- { 0x0000, 0x0000 },
- { 0x0104, 0xA213 },
- { 0x010E, 0x0058 },
- { 0x0111, 0x2000 },
- { 0x0322, 0xF7DF },
- { 0x1302, 0x12F7 },
- { 0x0336, 0x0010 },
- { 0x04FC, 0x35CF },
- { 0x0102, 0xed02 },
+};
What's this doing? Some of these (all that I checked) are normal registers not undocumented registers or anything and you're changing their default values in here as well as doing a reset. If we're setting values on normal registers we should at least do it through explicit, documented writes.
+#define RT1015_INIT_REG_LEN ARRAY_SIZE(init_list)
Not sure this define is worth the effort?
- rt1015->boost_mode = ucontrol->value.integer.value[0];
- if (rt1015->boost_mode == 0) {
/* Bypass */snd_soc_component_update_bits(component,RT1015_SMART_BST_CTRL1, RT1015_ABST_AUTO_EN_MASK |RT1015_ABST_FIX_TGT_MASK | RT1015_BYPASS_SWR_REG_MASK,RT1015_ABST_REG_MODE | RT1015_ABST_FIX_TGT_DIS |RT1015_BYPASS_SWRREG_BYPASS);- } else if (rt1015->boost_mode == 1) {
/* Adaptive */snd_soc_component_update_bits(component,RT1015_SMART_BST_CTRL1, RT1015_ABST_AUTO_EN_MASK |RT1015_ABST_FIX_TGT_MASK | RT1015_BYPASS_SWR_REG_MASK,RT1015_ABST_AUTO_MODE | RT1015_ABST_FIX_TGT_DIS |RT1015_BYPASS_SWRREG_PASS);- } else {
/* Fixed Adaptive */snd_soc_component_update_bits(component,RT1015_SMART_BST_CTRL1, RT1015_ABST_AUTO_EN_MASK |RT1015_ABST_FIX_TGT_MASK | RT1015_BYPASS_SWR_REG_MASK,RT1015_ABST_AUTO_MODE | RT1015_ABST_FIX_TGT_EN |RT1015_BYPASS_SWRREG_PASS);- }
This should be a switch statement and should reject invalid values.
- switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
if (rt1015->bypass_boost == 0) {snd_soc_component_write(component,RT1015_SYS_RST1, 0x05f7);snd_soc_component_write(component,RT1015_GAT_BOOST, 0xacfe);snd_soc_component_write(component,RT1015_PWR9, 0xaa00);snd_soc_component_write(component,RT1015_GAT_BOOST, 0xecfe);} else {
Do we need some sort of lock out to prevent applications changing the bypass boost mode while the amp is powered up?