Re: [PATCH v5 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 04, 2024 at 10:36:38PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
> 
> The ASoC driver enables I2S streaming to the device.
> 
> Signed-off-by: James Ogletree <jogletre@xxxxxxxxxxxxxxxxxxxxx>
> ---
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>

Need to also include bitfield.h for FIELD_PREP etc.

> +static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
> +			  struct snd_kcontrol *kcontrol,
> +			  int event)
> +{
> +	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
> +	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
> +	int ret;
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		ret = regmap_write(codec->regmap, CS40L50_DSP_QUEUE, CS40L50_STOP_PLAYBACK);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(codec->regmap, CS40L50_DSP_QUEUE, CS40L50_START_I2S);
> +		if (ret)
> +			return ret;
> +

Feels weird that we don't wait for these two commands to be
acknowledged by the DSP before doing the clock swap. Is that
intentional? Is the DSP just guaranteed to be so fast it doesn't
matter, in which case a comment would be nice.

> +static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> +	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
> +
> +	if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBS_CFS)
> +		return -EINVAL;
> +
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_NB_NF:
> +		codec->daifmt = 0;
> +		break;
> +	case SND_SOC_DAIFMT_NB_IF:
> +		codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
> +		break;
> +	case SND_SOC_DAIFMT_IB_IF:
> +		codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_I2S)
> +		codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);

It feels unlikely the chip supports all formats with no
additional settings? Probably should have a switch for the
supported formats and return an error.

> +static struct snd_soc_dai_driver cs40l50_dai[] = {
> +	{
> +		.name = "cs40l50-pcm",
> +		.id = 0,
> +		.playback = {
> +			.stream_name = "ASP Playback",
> +			.channels_min = 1,
> +			.channels_max = 2,
> +			.rates = SNDRV_PCM_RATE_48000,
> +			.formats = CS40L50_FORMATS,
> +		},
> +		.ops = &cs40l50_dai_ops,
> +		.symmetric_rate = 1,

The symmetric_rate feels a bit redundant since we only have
playback supported.

Thanks,
Charles




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux