On Fri, Sep 20, 2024 at 06:41:08PM +0300, Igor Prusov wrote:
> +config SND_SOC_ES7243E
> + tristate "Everest Semi ES7243E CODEC"
> +
This is an I2C device, it should depend on I2C.
> @@ -0,0 +1,693 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/**
> + * es7243e.c - ASoC Everest Semiconductor ES7243E audio ADC driver
> + *
Please make the enitre comment a C++ one so things look more
intentional.
> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
All rights reserved?
> +static const struct reg_sequence init_sequence[] = {
> + { ES7243E_CLK2, 0x00 },
> + { ES7243E_SDP, 0x00 },
> + { ES7243E_TDM, 0x00 },
> +
> + /* Set MCLK/LRCK ratio to 256 */
> + { ES7243E_ADC_OSR, 0x20 },
This should be dynamically configured rather than hard coded, provide
set_bclk_ratio().
> + /* Set ADC volume to 0dB */
> + { ES7243E_ADC_VOL, 0xbf },
Most things should use the chip defaults, especially things like volumes
- userspace can configure whatever it needs.
> +static int es7243e_suspend(struct snd_soc_component *component)
> +{
> + struct es7243e_priv *es7243e = snd_soc_component_get_drvdata(component);
> + int ret;
> + unsigned int val, mask;
> +
> + val = FIELD_PREP(ES7243E_SDP_MUTE, 0);
> + ret = snd_soc_component_update_bits(component, ES7243E_SDP,
> + ES7243E_SDP_MUTE, val);
> + if (ret < 0)
> + return ret;
> +
> + val = FIELD_PREP(ES7243E_PGA1_EN, 0);
> + snd_soc_component_update_bits(component, ES7243E_PGA1,
> + ES7243E_PGA1_EN, val);
> + if (ret < 0)
> + return ret;
> +
> + val = FIELD_PREP(ES7243E_PGA2_EN, 0);
> + snd_soc_component_update_bits(component, ES7243E_PGA2,
> + ES7243E_PGA2_EN, val);
> + if (ret < 0)
> + return ret;
This looks a lot like you should be using DAPM with events on the PGAs,
that will give power management at runtime as well.
> + ret = snd_soc_component_write(component, ES7243E_PDN, 0xff);
> + if (ret < 0)
> + return ret;
> +
> + mask = ES7243E_CLK1_ANA_ON | ES7243E_CLK1_ADC_ON;
> + ret = snd_soc_component_update_bits(component, ES7243E_CLK1, mask, 0);
> + if (ret < 0)
> + return ret;
> +
> + clk_disable_unprepare(es7243e->lrclk);
> + clk_disable_unprepare(es7243e->sclk);
You could also use set_bias_level() for chip level power, it doesn't
look like there's substantial delay.
> +static int es7243e_resume(struct snd_soc_component *component)
> +{
> + struct es7243e_priv *es7243e = snd_soc_component_get_drvdata(component);
> + int ret;
> + unsigned int val;
This doesn't resync the cache so the user volume settings will be lost.
Attachment:
signature.asc
Description: PGP signature
