Hi James, This one looks pretty good; just a few nits since a new series is coming anyway. On Fri, Mar 08, 2024 at 10:24:21PM +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> > --- > MAINTAINERS | 1 + > sound/soc/codecs/Kconfig | 11 ++ > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/cs40l50-codec.c | 307 +++++++++++++++++++++++++++++++ > 4 files changed, 321 insertions(+) > create mode 100644 sound/soc/codecs/cs40l50-codec.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 24cfb4f017bb..fca2454a7a38 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4943,6 +4943,7 @@ F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml > F: drivers/input/misc/cs40l* > F: drivers/mfd/cs40l* > F: include/linux/mfd/cs40l* > +F: sound/soc/codecs/cs40l* > > CIRRUS LOGIC DSP FIRMWARE DRIVER > M: Simon Trimmer <simont@xxxxxxxxxxxxxxxxxxxxx> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig > index f1e1dbc509f6..1a81bedfdbe3 100644 > --- a/sound/soc/codecs/Kconfig > +++ b/sound/soc/codecs/Kconfig > @@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS > imply SND_SOC_CS35L56_I2C > imply SND_SOC_CS35L56_SPI > imply SND_SOC_CS35L56_SDW > + imply SND_SOC_CS40L50 > imply SND_SOC_CS42L42 > imply SND_SOC_CS42L42_SDW > imply SND_SOC_CS42L43 > @@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW > help > Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control > > +config SND_SOC_CS40L50 > + tristate "Cirrus Logic CS40L50 CODEC" > + depends on MFD_CS40L50_CORE > + help > + This option enables support for I2S streaming to Cirrus Logic CS40L50. > + > + CS40L50 is a haptic driver with waveform memory, an integrated > + DSP, and closed-loop algorithms. If built as a module, it will be > + called snd-soc-cs40l50. > + > config SND_SOC_CS42L42_CORE > tristate > > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile > index a87e56938ce5..7e31f000774a 100644 > --- a/sound/soc/codecs/Makefile > +++ b/sound/soc/codecs/Makefile > @@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o > snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o > snd-soc-cs35l56-spi-objs := cs35l56-spi.o > snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o > +snd-soc-cs40l50-objs := cs40l50-codec.o > snd-soc-cs42l42-objs := cs42l42.o > snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o > snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o > @@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED) += snd-soc-cs35l56-shared.o > obj-$(CONFIG_SND_SOC_CS35L56_I2C) += snd-soc-cs35l56-i2c.o > obj-$(CONFIG_SND_SOC_CS35L56_SPI) += snd-soc-cs35l56-spi.o > obj-$(CONFIG_SND_SOC_CS35L56_SDW) += snd-soc-cs35l56-sdw.o > +obj-$(CONFIG_SND_SOC_CS40L50) += snd-soc-cs40l50.o > obj-$(CONFIG_SND_SOC_CS42L42_CORE) += snd-soc-cs42l42.o > obj-$(CONFIG_SND_SOC_CS42L42) += snd-soc-cs42l42-i2c.o > obj-$(CONFIG_SND_SOC_CS42L42_SDW) += snd-soc-cs42l42-sdw.o > diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c > new file mode 100644 > index 000000000000..23299b8dacfb > --- /dev/null > +++ b/sound/soc/codecs/cs40l50-codec.c > @@ -0,0 +1,307 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// CS40L50 Advanced Haptic Driver with waveform memory, > +// integrated DSP, and closed-loop algorithms > +// > +// Copyright 2024 Cirrus Logic, Inc. > +// > +// Author: James Ogletree <james.ogletree@xxxxxxxxxx> Typically we see a // C++ style comment for the SPDX identifier, and then /* block comments */ for the rest, unless the maintainer has explicitly asked for this style for this specific subsystem. The introductory block in the rest of the series is much more common. > + > +#include <linux/bitfield.h> > +#include <linux/mfd/cs40l50.h> > +#include <linux/pm_runtime.h> > +#include <sound/pcm_params.h> > +#include <sound/soc.h> > + > +#define CS40L50_REFCLK_INPUT 0x2C04 > +#define CS40L50_ASP_CONTROL2 0x4808 > +#define CS40L50_ASP_DATA_CONTROL5 0x4840 > + > +/* PLL Config */ > +#define CS40L50_PLL_REFCLK_BCLK 0x0 > +#define CS40L50_PLL_REFCLK_MCLK 0x5 > +#define CS40L50_PLL_REFCLK_LOOP_MASK BIT(11) > +#define CS40L50_PLL_REFCLK_OPEN_LOOP 1 > +#define CS40L50_PLL_REFCLK_CLOSED_LOOP 0 > +#define CS40L50_PLL_REFCLK_LOOP_SHIFT 11 > +#define CS40L50_PLL_REFCLK_FREQ_MASK GENMASK(10, 5) > +#define CS40L50_PLL_REFCLK_FREQ_SHIFT 5 > +#define CS40L50_PLL_REFCLK_SEL_MASK GENMASK(2, 0) > +#define CS40L50_BCLK_RATIO_DEFAULT 32 > + > +/* ASP Config */ > +#define CS40L50_ASP_RX_WIDTH_SHIFT 24 > +#define CS40L50_ASP_RX_WIDTH_MASK GENMASK(31, 24) > +#define CS40L50_ASP_RX_WL_MASK GENMASK(5, 0) > +#define CS40L50_ASP_FSYNC_INV_MASK BIT(2) > +#define CS40L50_ASP_BCLK_INV_MASK BIT(6) > +#define CS40L50_ASP_FMT_MASK GENMASK(10, 8) > +#define CS40L50_ASP_FMT_I2S 0x2 > + > +struct cs40l50_pll_config { > + unsigned int freq; > + unsigned int cfg; > +}; > + > +struct cs40l50_codec { > + struct device *dev; > + struct regmap *regmap; > + unsigned int daifmt; > + unsigned int bclk_ratio; > + unsigned int rate; > +}; > + > +static const struct cs40l50_pll_config cs40l50_pll_cfg[] = { > + {32768, 0x00}, > + {1536000, 0x1B}, > + {3072000, 0x21}, > + {6144000, 0x28}, > + {9600000, 0x30}, > + {12288000, 0x33}, Nit: please space these as is done elsewhere throughout the series, i.e.: { 12288000, 0x33 }, > +}; > + > +static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) { > + if (cs40l50_pll_cfg[i].freq == freq) { > + *cfg = cs40l50_pll_cfg[i].cfg; > + return 0; > + } > + } > + > + return -EINVAL; > +} > + > +static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src) > +{ > + unsigned int cfg; > + int ret; > + > + switch (clk_src) { > + case CS40L50_PLL_REFCLK_BCLK: > + ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg); > + if (ret) > + return ret; > + break; > + case CS40L50_PLL_REFCLK_MCLK: > + cfg = 0x00; 0x00 seems special; please consider #defining it. > + break; > + default: > + return -EINVAL; > + } > + > + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT, > + CS40L50_PLL_REFCLK_LOOP_MASK, > + CS40L50_PLL_REFCLK_OPEN_LOOP << > + CS40L50_PLL_REFCLK_LOOP_SHIFT); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT, > + CS40L50_PLL_REFCLK_FREQ_MASK | > + CS40L50_PLL_REFCLK_SEL_MASK, > + (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src); > + if (ret) > + return ret; > + > + return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT, > + CS40L50_PLL_REFCLK_LOOP_MASK, > + CS40L50_PLL_REFCLK_CLOSED_LOOP << > + CS40L50_PLL_REFCLK_LOOP_SHIFT); > +} > + > +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 = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK); > + if (ret) > + return ret; > + > + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S); > + if (ret) > + return ret; > + > + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK); > + if (ret) > + return ret; > + break; > + case SND_SOC_DAPM_PRE_PMD: > + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK); > + if (ret) > + return ret; > + break; Just for my own understanding, does the HALO core need to be told that I2S streaming is about to stop? Or does the L50 gracefully park the class D outputs when the PLL source is suddenly switched? > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = { > + SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en, > + SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), > + SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0), > + SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0), > + SND_SOC_DAPM_OUTPUT("OUT"), > +}; > + > +static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = { > + { "ASP Playback", NULL, "ASP PLL" }, > + { "ASPRX1", NULL, "ASP Playback" }, > + { "ASPRX2", NULL, "ASP Playback" }, > + > + { "OUT", NULL, "ASPRX1" }, > + { "OUT", NULL, "ASPRX2" }, > +}; > + > +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_CBC_CFC) > + 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: > + dev_err(codec->dev, "Invalid clock invert\n"); > + return -EINVAL; > + } > + > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { > + case SND_SOC_DAIFMT_I2S: > + codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S); > + break; > + default: > + dev_err(codec->dev, "Unsupported DAI format\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int cs40l50_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component); > + unsigned int asp_rx_wl = params_width(params); > + int ret; > + > + codec->rate = params_rate(params); > + > + ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5, > + CS40L50_ASP_RX_WL_MASK, asp_rx_wl); > + if (ret) > + return ret; > + > + codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT); > + > + return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2, > + CS40L50_ASP_FSYNC_INV_MASK | > + CS40L50_ASP_BCLK_INV_MASK | > + CS40L50_ASP_FMT_MASK | > + CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt); > +} > + > +static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) > +{ > + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component); > + > + codec->bclk_ratio = ratio; > + > + return 0; > +} > + > +static const struct snd_soc_dai_ops cs40l50_dai_ops = { > + .set_fmt = cs40l50_set_dai_fmt, > + .set_bclk_ratio = cs40l50_set_dai_bclk_ratio, > + .hw_params = cs40l50_hw_params, > +}; > + > +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 = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE, > + }, > + .ops = &cs40l50_dai_ops, > + }, > +}; > + > +static int cs40l50_codec_probe(struct snd_soc_component *component) > +{ > + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component); > + > + codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT; > + > + return 0; > +} > + > +static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = { > + .probe = cs40l50_codec_probe, > + .dapm_widgets = cs40l50_dapm_widgets, > + .num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets), > + .dapm_routes = cs40l50_dapm_routes, > + .num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes), > +}; > + > +static int cs40l50_codec_driver_probe(struct platform_device *pdev) > +{ > + struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent); > + struct cs40l50_codec *codec; > + > + codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL); > + if (!codec) > + return -ENOMEM; > + > + codec->regmap = cs40l50->regmap; > + codec->dev = &pdev->dev; > + > + return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50, > + cs40l50_dai, ARRAY_SIZE(cs40l50_dai)); > +} > + > +static const struct platform_device_id cs40l50_id[] = { > + { "cs40l50-codec", }, > + {} > +}; > +MODULE_DEVICE_TABLE(platform, cs40l50_id); > + > +static struct platform_driver cs40l50_codec_driver = { > + .probe = cs40l50_codec_driver_probe, > + .id_table = cs40l50_id, > + .driver = { > + .name = "cs40l50-codec", > + }, > +}; > +module_platform_driver(cs40l50_codec_driver); > + > +MODULE_DESCRIPTION("ASoC CS40L50 driver"); > +MODULE_AUTHOR("James Ogletree <james.ogletree@xxxxxxxxxx>"); > +MODULE_LICENSE("GPL"); > -- > 2.25.1 > Kind regards, Jeff LaBundy