public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] ASoC: codecs: add support for ES8389
@ 2025-03-27  8:14 Zhang Yi
  2025-03-27  8:14 ` [PATCH v6 1/2] " Zhang Yi
  2025-03-27  8:14 ` [PATCH v6 2/2] ASoC: dt-bindings: Add Everest ES8389 audio CODEC Zhang Yi
  0 siblings, 2 replies; 5+ messages in thread
From: Zhang Yi @ 2025-03-27  8:14 UTC (permalink / raw)
  To: broonie, robh, tiwai, devicetree, conor+dt, lgirdwood,
	linux-kernel, linux-sound, perex, krzk+dt
  Cc: amadeuszx.slawinski, krzk, Zhang Yi

The driver is for codec ES8389 of everest-semi.

v6 -> v5: add "Reviewed-by" at v6 2/2

v5 -> v4:
          - Modify kcontrol on DMIC in the codec driver
          - Modify the way to set the slot and add es8389_set_tdm_slot
           in the codec driver
          - Modify the problem ablout making the use of a MCLK depend on
           the configuration of a TDM slot in the codec driver
          - Remove the "everest,adc-slot", "everest,dac-slot" and "everest,dmic-enabled"
           in the codec driver and the yaml file, because they are unnecessary. 
          - Remove the "prefix_name" in the yaml file.

v4 -> v3:
          - Remove simple probe enter/exit debugs in the codec driver
          - Modify the problem about double free in i2c_probe
          - Modify the problem about coding style and wrapping
          - Modify the problem about Bindings are before the user
          - Modify the ref of "everest,adc-slot" and "everest,dac-slot"
             in the yaml file
          - Add description of "prefix_name" and "everest,dmic-enabled"
             in the yaml file

v3 -> v2: Modify the format of examples in the yaml file

v2 -> v1:
          - Remove a empty switch in the codec driver
          - Modify the format of examples in the yaml file

v1 -> v0:
          - Modify the register names in the codec driver 
          - Use yaml instead of txt to describe dts

Zhang Yi (2):
  ASoC: codecs: add support for ES8389
  ASoC: dt-bindings: Add Everest ES8389 audio CODEC

 .../bindings/sound/everest,es8389.yaml        |  50 +
 sound/soc/codecs/Kconfig                      |   7 +-
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/es8389.c                     | 966 ++++++++++++++++++
 sound/soc/codecs/es8389.h                     | 140 +++
 5 files changed, 1164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/sound/everest,es8389.yaml
 create mode 100644 sound/soc/codecs/es8389.c
 create mode 100644 sound/soc/codecs/es8389.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
* RE: [PATCH v6 1/2] ASoC: codecs: add support for ES8389
@ 2025-03-27  9:38 Zhang Yi
  0 siblings, 0 replies; 5+ messages in thread
From: Zhang Yi @ 2025-03-27  9:38 UTC (permalink / raw)
  To: broonie, robh, tiwai, devicetree, conor+dt, lgirdwood,
	linux-kernel, linux-sound, perex, krzk+dt
  Cc: amadeuszx.slawinski, krzk

> >   	imply SND_SOC_ES8316
> >   	imply SND_SOC_ES8323
> >   	imply SND_SOC_ES8326
> > +	imply SND_SOC_ES8389
> >   	imply SND_SOC_ES8328_SPI
> >   	imply SND_SOC_ES8328_I2C
> >   	imply SND_SOC_ES7134
> 
> Add it in alphabetical order?

I'll fix the issue about the order of this couple of places.

> > diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c new 
> > file mode 100644 index 000000000000..73c1966c30be
> > --- /dev/null
> > +++ b/sound/soc/codecs/es8389.c
> > @@ -0,0 +1,966 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * es8389.c  --  ES8389/ES8390 ALSA SoC Audio Codec
> 
> The only place that mentions ES8390? Is this correct?

Actually ES8389 and ES8390 are the same chip.I'll drop the ES8390 in this place
to avoid any misunderstandings.

> > +	{50, 400000, 8000, 0x00, 0x75, 0x05, 0xC8, 0x01, 0xC1, 0x90, 0x10, 0x00, 0x18, 0xC7, 0xD0, 0xC0, 0x8F, 0xC7, 0x01, 0x12, 0x00, 0x09, 0x19, 0x07},
> > +	{600, 4800000, 8000, 0x05, 0x65, 0x25, 0xF9, 0x00, 0xD1, 0x90, 0x10, 0x00, 0x18, 0xC7, 0xD0, 0xC0, 0x8F, 0xC7, 0x01, 0x12, 0x00, 0x09, 0x19, 0x07},
> > +	{1500, 12000000, 8000, 0x0E, 0x25, 0x25, 0xE8, 0x00, 0xD1, 0x90, 0x40, 0x00, 0x31, 0xC7, 0xC5, 0x00, 0x8F, 0xC7, 0x01, 0x12, 0x00, 0x09, 0x19, 0x07},
> > +	{2400, 19200000, 8000, 0x0B, 0x01, 0x00, 0xD0, 0x00, 0xD1, 0x80, 0x90, 0x00, 0x31, 0xC7, 0xC5, 0x00, 0xC7, 0xC7, 0x00, 0x12, 0x00, 0x09, 0x19, 0x07},
> > +	{3000, 24000000, 8000, 0x0E, 0x24, 0x05, 0xD0, 0x00, 0xC2, 0x80, 0xC0, 0x00, 0x31, 0xC7, 0xC5, 0x00, 0x8F, 0xC7, 0x01, 0x12, 0x00, 0x09, 0x19, 0x07},
> > +	{3250, 26000000, 8000, 0x40, 0x05, 0xA4, 0xC0, 0x00, 0xD1, 0x80, 
> > +0xD0, 0x00, 0x31, 0xC7, 0xC5, 0x00, 0xC7, 0xC7, 0x00, 0x12, 0x00, 
> > +0x09, 0x19, 0x07},
> > +
> > +};
> 
> Missing new line between array above and below function? I think checkpatch warns about this?

I'll fix it

> > +
> > +static int es8389_probe(struct snd_soc_component *codec) {
> > +	int ret = 0;
> > +	struct es8389_private *es8389 = 
> > +snd_soc_component_get_drvdata(codec);
> 
> Use "reverse christmas tree" notation for declaring variables? Also ret gets overwritten below, so it is useless to set it.

I'll fix it

> > +static const struct regmap_config es8389_regmap = {
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +
> > +	.max_register = ES8389_MAX_REGISTER,
> > +
> > +	.volatile_reg = es8389_volatile_register,
> > +	.cache_type = REGCACHE_MAPLE,
> > +};
> > +
> > +static void es8389_i2c_shutdown(struct i2c_client *i2c) {
> > +	struct snd_soc_component *component;
> > +	struct es8389_private *es8389;
> > +
> > +	es8389 = i2c_get_clientdata(i2c);
> > +	component = es8389->component;
> 
> Unused variable? And while you are removing it, you can also remove it from 'struct es8389_private', as this place seems to be the only user.

Yes, I'll drop them

> > +
> > +	regmap_write(es8389->regmap, ES8389_MASTER_MODE, 0x28);
> > +	regmap_write(es8389->regmap, ES8389_HPSW, 0x00);
> > +	regmap_write(es8389->regmap, ES8389_VMID, 0x00);
> > +	regmap_write(es8389->regmap, ES8389_RESET, 0x00);
> > +	regmap_write(es8389->regmap, ES8389_CSM_JUMP, 0xCC);
> > +	usleep_range(500000, 550000);//500MS
> > +	regmap_write(es8389->regmap, ES8389_CSM_JUMP, 0x00);
> > +	regmap_write(es8389->regmap, ES8389_ANA_CTL1, 0x08);
> > +	regmap_write(es8389->regmap, ES8389_ISO_CTL, 0xC1);
> > +	regmap_write(es8389->regmap, ES8389_PULL_DOWN, 0x00); }
> > +
> > +static int es8389_i2c_probe(struct i2c_client *i2c_client) {
> > +	struct es8389_private *es8389;
> > +	int ret = -1;
> 
> No need to set ret as it will be overwritten anyway, and '-1' is not considered a proper value. Use some error code if you must.

Ok

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-03-27  9:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-27  8:14 [PATCH v6 0/2] ASoC: codecs: add support for ES8389 Zhang Yi
2025-03-27  8:14 ` [PATCH v6 1/2] " Zhang Yi
2025-03-27  8:58   ` Amadeusz Sławiński
2025-03-27  8:14 ` [PATCH v6 2/2] ASoC: dt-bindings: Add Everest ES8389 audio CODEC Zhang Yi
  -- strict thread matches above, loose matches on Subject: below --
2025-03-27  9:38 [PATCH v6 1/2] ASoC: codecs: add support for ES8389 Zhang Yi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox