On Tue, Jan 14, 2020 at 10:56:28AM +0800, jack.yu@xxxxxxxxxxx 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?
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
