On Fri, Jul 15, 2022 at 01:41:00PM +0800, Zhu Ning wrote:
Looks mostly good but there's still some issues here, plus the ones Pierre found. Only one or two are substantial though, some of the things below are really minor:
Please check the coding style matches the kernel coding style - checkpatch will probably help here.
- snd_soc_dapm_mutex_unlock(dapm);
+} +static void es8326_jack_detect_handler(struct work_struct *work)
Blank line missing between functions.
- if(!es8326->jack)
goto out;
Missing space after the if.
+static int es8326_resume(struct snd_soc_component *component) +{
I'm not seeing anything in here to resync the register map with the device - the driver is using a register cache so that's going to break if the device looses power over suspend. TBH it's not clear to me that the driver isn't hard coding a specific set of paths...
- regmap_write(es8326->regmap, ES8326_INT_SOURCE_58, 0x08);
- regmap_write(es8326->regmap, ES8326_INTOUT_IO_59, 0x45);
Some of the hard coded register write sequences in here look a lot like they're assuming a specific board layout and might need more device tree configurability - what if the board doesn't use an interrupt or uses a different one?
- ret = device_property_read_u8(component->dev, "everest,mic1-src", &es8326->mic1_src);
- if (ret != 0) {
This is adding a DT binding but there's no DT binding document. Previous versions of this driver did have one, please include it with every submission.
--- /dev/null +++ b/sound/soc/codecs/es8326.h @@ -0,0 +1,187 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/*
- es8326.c -- es8326 ALSA SoC audio driver
Commend has the wrong filename here.