Messages in this thread | | | Date | Tue, 19 Sep 2023 13:00:57 +0100 | From | Mark Brown <> | Subject | Re: [PATCH v2 2/2] ASoC: codecs: Add Richtek rtq9128audio amplifier support |
| |
On Tue, Sep 19, 2023 at 09:18:21AM +0800, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com> > > Add Richtek rtq9128 automotive audio amplifier.
Looks mostly good, a few minor points below:
> --- /dev/null > +++ b/sound/soc/codecs/rtq9128.c > @@ -0,0 +1,742 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 Richtek Technology Corp.
Please make the entire block a C++ comment so things look more intentional.
> +static const struct regmap_config rtq9128_regmap_config = { > + .name = "rtq9128", > + .reg_bits = 8, > + .val_bits = 32, > + .val_format_endian = REGMAP_ENDIAN_BIG, > + .cache_type = REGCACHE_RBTREE,
_MAPLE is a better choice for most devices these days.
> + SOC_ENUM("CH1 SI Select", rtq9128_select_enum[0]), > + SOC_ENUM("CH2 SI Select", rtq9128_select_enum[1]), > + SOC_ENUM("CH3 SI Select", rtq9128_select_enum[2]), > + SOC_ENUM("CH4 SI Select", rtq9128_select_enum[3]), > + SOC_ENUM("PWM FREQ Select", rtq9128_select_enum[4]), > + SOC_ENUM("OUT2 Phase Select", rtq9128_select_enum[5]), > + SOC_ENUM("OUT3 Phase Select", rtq9128_select_enum[6]), > + SOC_ENUM("OUT4 Phase Select", rtq9128_select_enum[7]),
Don't use an array of enums with magic numbers like this, it's hard to read and maintain. Use individually named variables instead.
> + /* Turn channel state to Normal or HiZ */ > + ret = snd_soc_component_write_field(comp, RTQ9128_REG_STATE_CTRL, mask, > + event == SND_SOC_DAPM_POST_PMU ? 0 : 1);
The ternery operator could just be != here. [unhandled content-type:application/pgp-signature] | |