Re: [PATCH v2 2/2] ASoC: es8328: set SLRCK for the ES8388

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: es8328: missing ES8388 match entries in es8328-spi for SLRCK setup
--

commit db663b096dbe9f68fbdc51ad2211743c7793550e
Author: Bob Hui <bg9oxa@163.com>

ASoC: es8328: set SLRCK for the ES8388

This commit sets the SLRCK bit in ES8328_DACCONTROL21 during
es8328_component_probe() when the codec variant is ES8328_TYPE_ES8388 so that
the DAC and ADC share the single LRCK pin.

> diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
> index f04e310262e79..3d9a1fe1b9a00 100644
> --- a/sound/soc/codecs/es8328.c
> +++ b/sound/soc/codecs/es8328.c
> @@ -861,8 +861,25 @@ static int es8328_component_probe(struct snd_soc_component *component)
>  		goto clk_fail;
>  	}
>  
> +	/*
> +	 * The ES8388 has a single LRCK pin, so the DAC and the ADC must
> +	 * share it: without SLRCK the capture path never gets a frame
> +	 * clock and records digital silence while playback still works.
> +	 * Bit 7 of DAC Control 21 is SLRCK on the ES8388 and LD2MO (left
> +	 * DAC to the mono mixer) on the ES8328.
> +	 */
> +	if (es8328->type == ES8328_TYPE_ES8388) {
> +		ret = snd_soc_component_write(component, ES8328_DACCONTROL21,
> +					      ES8328_DACCONTROL21_SLRCK);
> +		if (ret)
> +			goto err_slrck;
> +	}

[Severity: Medium]
Can es8328->type ever be ES8328_TYPE_ES8388 when the codec is connected over
SPI?

In sound/soc/codecs/es8328-spi.c, es8328_spi_probe() checks for
ES8328_TYPE_ES8388 and passes the resolved type to es8328_probe():

    enum es8328_type type = (unsigned long)spi_get_device_match_data(spi);
    const struct regmap_config *config;

    config = type == ES8328_TYPE_ES8388 ? &es8388_regmap_config
                                        : &es8328_regmap_config;

    return es8328_probe(&spi->dev, devm_regmap_init_spi(spi, config), type);

However, es8328_of_match in sound/soc/codecs/es8328-spi.c only lists
"everest,es8328" and es8328_spi_driver has no spi_device_id table:

    static const struct of_device_id es8328_of_match[] = {
            { .compatible = "everest,es8328", .data = (void *)ES8328_TYPE_ES8328 },
            { }
    };

When an ES8388 is connected over SPI (with devicetree compatible
"everest,es8388", "everest,es8328"), it matches the "everest,es8328" fallback
entry with ES8328_TYPE_ES8328, so es8328_component_probe() will skip setting
ES8328_DACCONTROL21_SLRCK.

Should sound/soc/codecs/es8328-spi.c also include the "everest,es8388" match
entry (and corresponding SPI device IDs) to match sound/soc/codecs/es8328-i2c.c?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031-es8328-vol-init-v2-0-d15b04c22f4f@163.com?part=2