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.
Attachment:
signature.asc
Description: PGP signature
