On Mon, May 15, 2023 at 08:40:19AM +0100, Aidan MacDonald wrote:
+++ b/sound/soc/codecs/ess-es9218p.c @@ -0,0 +1,805 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2022-2023 Aidan MacDonald
- */
Please make the entire comment a C++ one so things look more intentional.
+enum es9218p_supply_id {
- ES9218P_SUPPLY_AVDD,
- ES9218P_SUPPLY_VCCA,
- ES9218P_SUPPLY_AVCC3V3,
- ES9218P_SUPPLY_AVCC1V8,
- ES9218P_NUM_SUPPLIES
+};
+static const char * const es9218p_supply_names[ES9218P_NUM_SUPPLIES] = {
- "avdd",
- "vcca",
- "avcc3v3",
- "avcc1v8",
+};
These could easily get out of sync, it would be better to explicitly assign the names to the slots identified by the constants
[ES9218P_SUPPLY_VCCA] = "vcca",
for example.
+static int es9218p_wide_write(struct regmap *regmap, unsigned int reg,
int count, unsigned int value)+{
- u8 data[4];
- int i;
- for (i = 0; i < count; i++) {
data[i] = value & 0xff;value >>= 8;- }
This needs a bounds check to make sure we don't overflow data.
+static int outlevel_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
- struct es9218p_priv *priv = snd_soc_component_get_drvdata(component);
- priv->output_2v = ucontrol->value.enumerated.item[0];
- es9218p_update_amp_mode(component);
- return 1;
+}
Running the mixer-test selftest on a card with this driver will report that the driver generates spurious events when there is no change in value and doesn't validate input. Similar issues apply to the other enums.