All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver
@ 2020-05-18  2:17 Sia Jee Heng
  2020-05-18  2:17 ` [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver Sia Jee Heng
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Sia Jee Heng @ 2020-05-18  2:17 UTC (permalink / raw)
  To: alsa-devel; +Cc: liam.r.girdwood, broonie, tiwai, pierre-louis.bossart

The below series of patches support the KeemBay ASoC driver.
It enabled the tlv320aic3204 machine driver and the platform driver initialize
the i2s to capture and playback the pcm data on the ARM. The i2s is running
in polling mode.

There is no DSP in the KeemBay SoC. Users are rely on the Gstreamer plugin
to perform some Audio preprocessing.

Change History:
v2:
- Corrected I2S naming for DT binding.
  
v1:
- Initial version.

Sia Jee Heng (4):
  ASoC: Intel: Add KeemBay platform driver
  ASoC: Intel: Boards: Add KeemBay machine driver
  ASoC: Intel: Add makefiles and kconfig changes for KeemBay
  dt-bindings: sound: Add documentation for KeemBay sound card and i2s

 .../bindings/sound/intel,keembay-i2s.yaml          |  57 ++
 .../bindings/sound/intel,keembay-sound-card.yaml   |  30 +
 sound/soc/intel/Kconfig                            |   7 +
 sound/soc/intel/Makefile                           |   1 +
 sound/soc/intel/boards/Kconfig                     |  15 +
 sound/soc/intel/boards/Makefile                    |   4 +
 sound/soc/intel/boards/kmb_tlv3204.c               | 144 ++++
 sound/soc/intel/keembay/Makefile                   |   4 +
 sound/soc/intel/keembay/kmb_platform.c             | 746 +++++++++++++++++++++
 sound/soc/intel/keembay/kmb_platform.h             | 145 ++++
 10 files changed, 1153 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/intel,keembay-sound-card.yaml
 create mode 100644 sound/soc/intel/boards/kmb_tlv3204.c
 create mode 100644 sound/soc/intel/keembay/Makefile
 create mode 100644 sound/soc/intel/keembay/kmb_platform.c
 create mode 100644 sound/soc/intel/keembay/kmb_platform.h

-- 
1.9.1


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

* [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver
  2020-05-18  2:17 [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Sia Jee Heng
@ 2020-05-18  2:17 ` Sia Jee Heng
  2020-05-18 17:06   ` Mark Brown
  2020-05-18  2:17 ` [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver Sia Jee Heng
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Sia Jee Heng @ 2020-05-18  2:17 UTC (permalink / raw)
  To: alsa-devel; +Cc: liam.r.girdwood, broonie, tiwai, pierre-louis.bossart

Add KeemBay ASoC platform driver which initialize the i2s controller
and uses i2s to capture and transmit pcm data to external codec.
The i2s is running in polling mode.

Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
Signed-off-by: Sia Jee Heng <jee.heng.sia@intel.com>
---
 sound/soc/intel/keembay/Makefile       |   4 +
 sound/soc/intel/keembay/kmb_platform.c | 746 +++++++++++++++++++++++++++++++++
 sound/soc/intel/keembay/kmb_platform.h | 145 +++++++
 3 files changed, 895 insertions(+)
 create mode 100644 sound/soc/intel/keembay/Makefile
 create mode 100644 sound/soc/intel/keembay/kmb_platform.c
 create mode 100644 sound/soc/intel/keembay/kmb_platform.h

diff --git a/sound/soc/intel/keembay/Makefile b/sound/soc/intel/keembay/Makefile
new file mode 100644
index 0000000..9084e8c
--- /dev/null
+++ b/sound/soc/intel/keembay/Makefile
@@ -0,0 +1,4 @@
+snd-soc-kmb_platform-objs := \
+	        kmb_platform.o
+
+obj-$(CONFIG_SND_SOC_INTEL_KEEMBAY) += snd-soc-kmb_platform.o
diff --git a/sound/soc/intel/keembay/kmb_platform.c b/sound/soc/intel/keembay/kmb_platform.c
new file mode 100644
index 0000000..49a5b0d
--- /dev/null
+++ b/sound/soc/intel/keembay/kmb_platform.c
@@ -0,0 +1,746 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Intel KeemBay Platform driver
+ *
+ *  Copyright (C) 2020 Intel Corporation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include "kmb_platform.h"
+
+#define PERIODS_MIN		2
+#define PERIODS_MAX		48
+#define PERIOD_BYTES_MIN	4096
+#define BUFFER_BYTES_MAX	(PERIODS_MAX * PERIOD_BYTES_MIN)
+#define TDM_OPERATION		1
+#define I2S_OPERATION		0
+#define DATA_WIDTH_CONFIG_BIT	6
+#define TDM_CHANNEL_CONFIG_BIT	3
+#define I2S_SAMPLE_RATES	(SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_48000)
+
+/* Array subscript */
+#define I2S_MASTER_MODE		0
+#define I2S_SLAVE_MODE		1
+
+static const char * const i2s_mode_name[] = {"master", "slave"};
+
+/* Maximum bit resolution of a channel - not uniformly spaced */
+static const u32 fifo_width[COMP_MAX_WORDSIZE] = {12, 16, 20, 24, 32,};
+
+/* PCM format to support channel resolution */
+static const u32 formats[COMP_MAX_WORDSIZE] = {
+	SNDRV_PCM_FMTBIT_S16_LE,
+	SNDRV_PCM_FMTBIT_S16_LE,
+	SNDRV_PCM_FMTBIT_S24_LE,
+	SNDRV_PCM_FMTBIT_S24_LE,
+	SNDRV_PCM_FMTBIT_S32_LE,
+};
+
+static const struct snd_pcm_hardware kmb_pcm_hardware = {
+	.info = SNDRV_PCM_INFO_INTERLEAVED |
+		SNDRV_PCM_INFO_MMAP |
+		SNDRV_PCM_INFO_MMAP_VALID |
+		SNDRV_PCM_INFO_BATCH |
+		SNDRV_PCM_INFO_BLOCK_TRANSFER,
+	.rates = I2S_SAMPLE_RATES,
+	.rate_min = 16000,
+	.rate_max = 48000,
+	.formats = SNDRV_PCM_FMTBIT_S16_LE |
+		   SNDRV_PCM_FMTBIT_S24_LE |
+		   SNDRV_PCM_FMTBIT_S32_LE,
+	.channels_min = 2,
+	.channels_max = 2,
+	.buffer_bytes_max = BUFFER_BYTES_MAX,
+	.period_bytes_min = PERIOD_BYTES_MIN,
+	.period_bytes_max = BUFFER_BYTES_MAX / PERIODS_MIN,
+	.periods_min = PERIODS_MIN,
+	.periods_max = PERIODS_MAX,
+	.fifo_size = 16,
+};
+
+static unsigned int pcm_tx_fn(struct kmb_i2s_info *dev,
+			      struct snd_pcm_runtime *runtime,
+			      unsigned int tx_ptr, bool *period_elapsed)
+{
+	unsigned int period_pos = tx_ptr % runtime->period_size;
+	u16(*p16)[2];
+	u32(*p32)[2];
+	int i;
+
+	if (dev->config.data_width == 16)
+		p16 = (void *)runtime->dma_area;
+	else
+		p32 = (void *)runtime->dma_area;
+	/* KMB i2s uses two separate L/R FIFO */
+	for (i = 0; i < dev->fifo_th; i++) {
+		if (dev->config.data_width == 16) {
+			writel(p16[tx_ptr][0], dev->i2s_base + LRBR_LTHR(0));
+			writel(p16[tx_ptr][1], dev->i2s_base + RRBR_RTHR(0));
+		} else {
+			writel(p32[tx_ptr][0], dev->i2s_base + LRBR_LTHR(0));
+			writel(p32[tx_ptr][1], dev->i2s_base + RRBR_RTHR(0));
+		}
+
+		period_pos++;
+
+		if (++tx_ptr >= runtime->buffer_size)
+			tx_ptr = 0;
+	}
+
+	*period_elapsed = period_pos >= runtime->period_size;
+
+	return tx_ptr;
+}
+
+static unsigned int pcm_rx_fn(struct kmb_i2s_info *dev,
+			      struct snd_pcm_runtime *runtime,
+			      unsigned int rx_ptr, bool *period_elapsed)
+{
+	unsigned int period_pos = rx_ptr % runtime->period_size;
+	u16(*p16)[2];
+	u32(*p32)[2];
+	int i;
+
+	if (dev->config.data_width == 16)
+		p16 = (void *)runtime->dma_area;
+	else
+		p32 = (void *)runtime->dma_area;
+	/* KMB i2s uses two separate L/R FIFO */
+	for (i = 0; i < dev->fifo_th; i++) {
+		if (dev->config.data_width == 16) {
+			p16[rx_ptr][0] = readl(dev->i2s_base + LRBR_LTHR(0));
+			p16[rx_ptr][1] = readl(dev->i2s_base + RRBR_RTHR(0));
+		} else {
+			p32[rx_ptr][0] = readl(dev->i2s_base + LRBR_LTHR(0));
+			p32[rx_ptr][1] = readl(dev->i2s_base + RRBR_RTHR(0));
+		}
+
+		period_pos++;
+
+		if (++rx_ptr >= runtime->buffer_size)
+			rx_ptr = 0;
+	}
+
+	*period_elapsed = period_pos >= runtime->period_size;
+
+	return rx_ptr;
+}
+
+static inline void i2s_disable_channels(struct kmb_i2s_info *kmb_i2s,
+					u32 stream)
+{
+	struct i2s_clk_config_data *config = &kmb_i2s->config;
+	u32 i;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		for (i = 0; i < config->chan_nr / 2; i++)
+			writel(0, kmb_i2s->i2s_base + TER(i));
+	} else {
+		for (i = 0; i < config->chan_nr / 2; i++)
+			writel(0, kmb_i2s->i2s_base + RER(i));
+	}
+}
+
+static inline void i2s_clear_irqs(struct kmb_i2s_info *kmb_i2s, u32 stream)
+{
+	struct i2s_clk_config_data *config = &kmb_i2s->config;
+	u32 i;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		for (i = 0; i < config->chan_nr / 2; i++)
+			readl(kmb_i2s->i2s_base + TOR(i));
+	} else {
+		for (i = 0; i < config->chan_nr / 2; i++)
+			readl(kmb_i2s->i2s_base + ROR(i));
+	}
+}
+
+static inline void i2s_irq_trigger(struct kmb_i2s_info *kmb_i2s, u32 stream,
+				   int chan_nr, bool trigger)
+{
+	u32 i, irq;
+	u32 flag;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		flag = TX_INT_FLAG;
+	else
+		flag = RX_INT_FLAG;
+
+	for (i = 0; i < chan_nr / 2; i++) {
+		irq = readl(kmb_i2s->i2s_base + IMR(i));
+		irq = trigger ? irq & ~flag : irq | flag;
+		writel(irq, kmb_i2s->i2s_base + IMR(i));
+	}
+}
+
+static void pcm_operation(struct kmb_i2s_info *kmb_i2s, bool playback)
+{
+	struct snd_pcm_substream *substream;
+	bool active, period_elapsed;
+
+	if (playback)
+		substream = kmb_i2s->tx_substream;
+	else
+		substream = kmb_i2s->rx_substream;
+
+	active = substream && snd_pcm_running(substream);
+
+	if (active) {
+		unsigned int ptr;
+		unsigned int new_ptr;
+
+		if (playback) {
+			ptr = kmb_i2s->tx_ptr;
+			new_ptr = pcm_tx_fn(kmb_i2s, substream->runtime,
+					    ptr, &period_elapsed);
+			cmpxchg(&kmb_i2s->tx_ptr, ptr, new_ptr);
+		} else {
+			ptr = kmb_i2s->rx_ptr;
+			new_ptr = pcm_rx_fn(kmb_i2s, substream->runtime,
+					    ptr, &period_elapsed);
+			cmpxchg(&kmb_i2s->rx_ptr, ptr, new_ptr);
+		}
+
+		if (period_elapsed)
+			snd_pcm_period_elapsed(substream);
+	}
+}
+
+static int kmb_pcm_open(struct snd_soc_component *component,
+			struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct kmb_i2s_info *kmb_i2s;
+
+	kmb_i2s = snd_soc_dai_get_drvdata(asoc_rtd_to_cpu(rtd, 0));
+	snd_soc_set_runtime_hwparams(substream, &kmb_pcm_hardware);
+	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
+	runtime->private_data = kmb_i2s;
+
+	return 0;
+}
+
+static int kmb_pcm_trigger(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct kmb_i2s_info *kmb_i2s = runtime->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			kmb_i2s->tx_ptr = 0;
+			kmb_i2s->tx_substream = substream;
+		} else {
+			kmb_i2s->rx_ptr = 0;
+			kmb_i2s->rx_substream = substream;
+		}
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			kmb_i2s->tx_substream = NULL;
+		else
+			kmb_i2s->rx_substream = NULL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static irqreturn_t i2s_irq_handler(int irq, void *dev_id)
+{
+	struct kmb_i2s_info *kmb_i2s = dev_id;
+	struct i2s_clk_config_data *config = &kmb_i2s->config;
+	irqreturn_t ret = IRQ_NONE;
+	u32 isr[4];
+	int i;
+
+	for (i = 0; i < config->chan_nr / 2; i++)
+		isr[i] = readl(kmb_i2s->i2s_base + ISR(i));
+
+	i2s_clear_irqs(kmb_i2s, SNDRV_PCM_STREAM_PLAYBACK);
+	i2s_clear_irqs(kmb_i2s, SNDRV_PCM_STREAM_CAPTURE);
+
+	for (i = 0; i < config->chan_nr / 2; i++) {
+		/*
+		 * Check if TX fifo is empty. If empty fill FIFO with samples
+		 */
+		if ((isr[i] & ISR_TXFE)) {
+			pcm_operation(kmb_i2s, true);
+			ret = IRQ_HANDLED;
+		}
+		/*
+		 * Data available. Retrieve samples from FIFO
+		 */
+		if ((isr[i] & ISR_RXDA)) {
+			pcm_operation(kmb_i2s, false);
+			ret = IRQ_HANDLED;
+		}
+		/* Error Handling: TX */
+		if (isr[i] & ISR_TXFO) {
+			dev_dbg(kmb_i2s->dev, "TX overrun (ch_id=%d)\n", i);
+			ret = IRQ_HANDLED;
+		}
+		/* Error Handling: RX */
+		if (isr[i] & ISR_RXFO) {
+			dev_dbg(kmb_i2s->dev, "RX overrun (ch_id=%d)\n", i);
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	return ret;
+}
+
+static int kmb_platform_pcm_new(struct snd_soc_component *component,
+				struct snd_soc_pcm_runtime *soc_runtime)
+{
+	size_t size = kmb_pcm_hardware.buffer_bytes_max;
+	/* Use SNDRV_DMA_TYPE_CONTINUOUS as KMB doesn't use PCI sg buffer */
+	snd_pcm_set_managed_buffer_all(soc_runtime->pcm,
+				       SNDRV_DMA_TYPE_CONTINUOUS,
+				       NULL, size, size);
+	return 0;
+}
+
+static snd_pcm_uframes_t kmb_pcm_pointer(struct snd_soc_component *component,
+					 struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct kmb_i2s_info *kmb_i2s = runtime->private_data;
+	snd_pcm_uframes_t pos;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		pos = kmb_i2s->tx_ptr;
+	else
+		pos = kmb_i2s->rx_ptr;
+
+	return pos < runtime->buffer_size ? pos : 0;
+}
+
+static const struct snd_soc_component_driver kmb_component = {
+	.name		= "kmb",
+	.pcm_construct	= kmb_platform_pcm_new,
+	.open		= kmb_pcm_open,
+	.trigger	= kmb_pcm_trigger,
+	.pointer	= kmb_pcm_pointer,
+};
+
+static void i2s_start(struct kmb_i2s_info *kmb_i2s,
+		      struct snd_pcm_substream *substream)
+{
+	struct i2s_clk_config_data *config = &kmb_i2s->config;
+
+	/* I2S Programming sequence in Keem_Bay_VPU_DB_v1.1 */
+	writel(1, kmb_i2s->i2s_base + IER);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		writel(1, kmb_i2s->i2s_base + ITER);
+	else
+		writel(1, kmb_i2s->i2s_base + IRER);
+
+	i2s_irq_trigger(kmb_i2s, substream->stream, config->chan_nr, true);
+
+	if (kmb_i2s->master)
+		writel(1, kmb_i2s->i2s_base + CER);
+	else
+		writel(0, kmb_i2s->i2s_base + CER);
+}
+
+static void i2s_stop(struct kmb_i2s_info *kmb_i2s,
+		     struct snd_pcm_substream *substream)
+{
+	/* I2S Programming sequence in Keem_Bay_VPU_DB_v1.1 */
+	i2s_clear_irqs(kmb_i2s, substream->stream);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		writel(0, kmb_i2s->i2s_base + ITER);
+	else
+		writel(0, kmb_i2s->i2s_base + IRER);
+
+	i2s_irq_trigger(kmb_i2s, substream->stream, 8, false);
+
+	if (!kmb_i2s->active) {
+		writel(0, kmb_i2s->i2s_base + CER);
+		writel(0, kmb_i2s->i2s_base + IER);
+	}
+}
+
+static int kmb_dai_trigger(struct snd_pcm_substream *substream,
+			   int cmd, struct snd_soc_dai *cpu_dai)
+{
+	struct kmb_i2s_info *kmb_i2s  = snd_soc_dai_get_drvdata(cpu_dai);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		/* Keep track of i2s activity before turn off
+		 * the i2s interface
+		 */
+		kmb_i2s->active++;
+		i2s_start(kmb_i2s, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		kmb_i2s->active--;
+		i2s_stop(kmb_i2s, substream);
+		break;
+	default:
+		return  -EINVAL;
+	}
+
+	return 0;
+}
+
+static void i2s_config(struct kmb_i2s_info *kmb_i2s, int stream)
+{
+	struct i2s_clk_config_data *config = &kmb_i2s->config;
+	u32 ch_reg;
+
+	i2s_disable_channels(kmb_i2s, stream);
+
+	for (ch_reg = 0; ch_reg < config->chan_nr / 2; ch_reg++) {
+		if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			writel(kmb_i2s->xfer_resolution,
+			       kmb_i2s->i2s_base + TCR(ch_reg));
+
+			writel(kmb_i2s->fifo_th - 1,
+			       kmb_i2s->i2s_base + TFCR(ch_reg));
+
+			writel(1, kmb_i2s->i2s_base + TER(ch_reg));
+		} else {
+			writel(kmb_i2s->xfer_resolution,
+			       kmb_i2s->i2s_base + RCR(ch_reg));
+
+			writel(kmb_i2s->fifo_th - 1,
+			       kmb_i2s->i2s_base + RFCR(ch_reg));
+
+			writel(1, kmb_i2s->i2s_base + RER(ch_reg));
+		}
+	}
+}
+
+static int kmb_dai_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *hw_params,
+			     struct snd_soc_dai *cpu_dai)
+{
+	struct kmb_i2s_info *kmb_i2s = snd_soc_dai_get_drvdata(cpu_dai);
+	struct i2s_clk_config_data *config = &kmb_i2s->config;
+	u32 register_val, write_val;
+	int ret;
+
+	switch (params_format(hw_params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		config->data_width = 16;
+		kmb_i2s->ccr = 0x00;
+		kmb_i2s->xfer_resolution = 0x02;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		config->data_width = 24;
+		kmb_i2s->ccr = 0x08;
+		kmb_i2s->xfer_resolution = 0x04;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		config->data_width = 32;
+		kmb_i2s->ccr = 0x10;
+		kmb_i2s->xfer_resolution = 0x05;
+		break;
+	default:
+		dev_err(kmb_i2s->dev, "kmb: unsupported PCM fmt");
+		return -EINVAL;
+	}
+
+	config->chan_nr = params_channels(hw_params);
+
+	switch (config->chan_nr) {
+	/* TODO: This switch case will handle up to TDM8 in the near future */
+	case TWO_CHANNEL_SUPPORT:
+		write_val = ((config->chan_nr / 2) << TDM_CHANNEL_CONFIG_BIT) |
+				(config->data_width << DATA_WIDTH_CONFIG_BIT) |
+				MASTER_MODE | I2S_OPERATION;
+
+		writel(write_val, kmb_i2s->pss_base + I2S_GEN_CFG_0);
+
+		register_val = readl(kmb_i2s->pss_base + I2S_GEN_CFG_0);
+		dev_dbg(kmb_i2s->dev, "pss register = 0x%X", register_val);
+		break;
+	default:
+		dev_dbg(kmb_i2s->dev, "channel not supported\n");
+		return -EINVAL;
+	}
+
+	i2s_config(kmb_i2s, substream->stream);
+
+	writel(kmb_i2s->ccr, kmb_i2s->i2s_base + CCR);
+
+	config->sample_rate = params_rate(hw_params);
+
+	if (kmb_i2s->master) {
+		/* Only 2 ch supported in Master mode */
+		u32 bitclk = config->sample_rate * config->data_width * 2;
+
+		ret = clk_set_rate(kmb_i2s->clk_i2s, bitclk);
+		if (ret) {
+			dev_err(kmb_i2s->dev,
+				"Can't set I2S clock rate: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int kmb_dai_prepare(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *cpu_dai)
+{
+	struct kmb_i2s_info *kmb_i2s = snd_soc_dai_get_drvdata(cpu_dai);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		writel(1, kmb_i2s->i2s_base + TXFFR);
+	else
+		writel(1, kmb_i2s->i2s_base + RXFFR);
+
+	return 0;
+}
+
+static struct snd_soc_dai_ops kmb_dai_ops = {
+	.trigger	= kmb_dai_trigger,
+	.hw_params	= kmb_dai_hw_params,
+	.prepare	= kmb_dai_prepare,
+};
+
+static struct snd_soc_dai_driver intel_kmb_platform_dai[] = {
+	{
+		.name = "kmb-plat-dai",
+		.playback = {
+			.channels_min = 2,
+			.channels_max = 2,
+			.rates = I2S_SAMPLE_RATES,
+			.rate_min = 16000,
+			.rate_max = 48000,
+			.formats = (SNDRV_PCM_FMTBIT_S32_LE |
+				    SNDRV_PCM_FMTBIT_S24_LE |
+				    SNDRV_PCM_FMTBIT_S16_LE),
+		},
+		.capture = {
+			.channels_min = 2,
+			.channels_max = 2,
+			.rates = I2S_SAMPLE_RATES,
+			.rate_min = 16000,
+			.rate_max = 48000,
+			.formats = (SNDRV_PCM_FMTBIT_S32_LE |
+				    SNDRV_PCM_FMTBIT_S24_LE |
+				    SNDRV_PCM_FMTBIT_S16_LE),
+		},
+		.ops = &kmb_dai_ops,
+	},
+};
+
+static int kmb_configure_dai(struct kmb_i2s_info *kmb_i2s,
+			     struct snd_soc_dai_driver *kmb_i2s_dai,
+			     unsigned int rates)
+{
+	/*
+	 * Read component parameter registers to extract
+	 * the I2S block's configuration.
+	 */
+	u32 comp1 = readl(kmb_i2s->i2s_base + kmb_i2s->i2s_reg_comp1);
+	u32 comp2 = readl(kmb_i2s->i2s_base + kmb_i2s->i2s_reg_comp2);
+	u32 fifo_depth = 1 << COMP1_FIFO_DEPTH(comp1);
+	u32 idx;
+
+	if (COMP1_TX_ENABLED(comp1)) {
+		idx = COMP1_TX_WORDSIZE_0(comp1);
+		kmb_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM;
+		kmb_i2s_dai->playback.channels_max =
+				1 << COMP1_TX_CHANNELS(comp1);
+		kmb_i2s_dai->playback.formats = formats[idx];
+		kmb_i2s_dai->playback.rates = rates;
+	}
+
+	if (COMP1_RX_ENABLED(comp1)) {
+		idx = COMP2_RX_WORDSIZE_0(comp2);
+		kmb_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM;
+		kmb_i2s_dai->capture.channels_max =
+				1 << COMP1_RX_CHANNELS(comp1);
+		kmb_i2s_dai->capture.formats = formats[idx];
+		kmb_i2s_dai->capture.rates = rates;
+	}
+
+	if (COMP1_MODE_EN(comp1)) {
+		dev_dbg(kmb_i2s->dev, "kmb: i2s master mode supported\n");
+		kmb_i2s->capability |= DW_I2S_MASTER;
+	} else {
+		dev_dbg(kmb_i2s->dev, "kmb: i2s slave mode supported\n");
+		kmb_i2s->capability |= DW_I2S_SLAVE;
+	}
+
+	kmb_i2s->fifo_th = fifo_depth / 2;
+
+	return 0;
+}
+
+static int kmb_configure_dai_by_dt(struct kmb_i2s_info *kmb_i2s,
+				   struct snd_soc_dai_driver *kmb_i2s_dai)
+{
+	u32 comp1 = readl(kmb_i2s->i2s_base + I2S_COMP_PARAM_1);
+
+	if (COMP1_TX_ENABLED(comp1))
+		kmb_i2s->capability |= DWC_I2S_PLAY;
+
+	if (COMP1_RX_ENABLED(comp1))
+		kmb_i2s->capability |= DWC_I2S_RECORD;
+
+	return kmb_configure_dai(kmb_i2s, kmb_i2s_dai,
+		I2S_SAMPLE_RATES);
+}
+
+static void kmb_disable_clk(void *data)
+{
+	clk_disable_unprepare(data);
+}
+
+static int kmb_plat_dai_probe(struct platform_device *pdev)
+{
+	struct snd_soc_dai_driver *kmb_i2s_dai;
+	struct device *dev = &pdev->dev;
+	struct kmb_i2s_info *kmb_i2s;
+	const char *i2s_mode;
+	int ret, irq;
+
+	kmb_i2s = devm_kzalloc(dev, sizeof(*kmb_i2s), GFP_KERNEL);
+	if (!kmb_i2s)
+		return -ENOMEM;
+
+	kmb_i2s_dai = devm_kzalloc(dev, sizeof(*kmb_i2s_dai), GFP_KERNEL);
+	if (!kmb_i2s_dai)
+		return -ENOMEM;
+
+	kmb_i2s_dai->ops = &kmb_dai_ops;
+
+	kmb_i2s->clk_apb = devm_clk_get(dev, "apb_clk");
+	if (IS_ERR(kmb_i2s->clk_apb)) {
+		dev_err(dev, "Failed to get apb clock\n");
+		return PTR_ERR(kmb_i2s->clk_apb);
+	}
+
+	/* Enable apb clk prior to access to the registers */
+	ret = clk_prepare_enable(kmb_i2s->clk_apb);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, kmb_disable_clk, kmb_i2s->clk_apb);
+	if (ret) {
+		dev_err(dev, "Failed to add clk_apb reset action\n");
+		return ret;
+	}
+
+	kmb_i2s->i2s_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(kmb_i2s->i2s_base))
+		return PTR_ERR(kmb_i2s->i2s_base);
+
+	kmb_i2s->pss_base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(kmb_i2s->pss_base))
+		return PTR_ERR(kmb_i2s->pss_base);
+
+	kmb_i2s->dev = &pdev->dev;
+
+	irq = platform_get_irq_optional(pdev, 0);
+	if (irq > 0) {
+		ret = devm_request_irq(dev, irq, i2s_irq_handler, 0,
+				       pdev->name, kmb_i2s);
+		if (ret < 0) {
+			dev_err(dev, "failed to request irq\n");
+			return ret;
+		}
+	}
+
+	ret = of_property_read_string(dev->of_node, "mode", &i2s_mode);
+	if (ret < 0) {
+		dev_err(dev, "Couldn't find the entry\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(kmb_i2s->dev, "Mode = %s", i2s_mode);
+
+	ret = match_string(i2s_mode_name, ARRAY_SIZE(i2s_mode_name), i2s_mode);
+
+	switch (ret) {
+	case I2S_MASTER_MODE:
+		kmb_i2s->master = true;
+		/* Set the mode prior to access to the rest of the register */
+		writel(MASTER_MODE, kmb_i2s->pss_base + I2S_GEN_CFG_0);
+		break;
+	case I2S_SLAVE_MODE:
+		kmb_i2s->master = false;
+		break;
+	default:
+		dev_err(dev, "invalid i2s mode '%s'\n", i2s_mode);
+		return ret;
+	}
+
+	kmb_i2s->i2s_reg_comp1 = I2S_COMP_PARAM_1;
+	kmb_i2s->i2s_reg_comp2 = I2S_COMP_PARAM_2;
+
+	ret = kmb_configure_dai_by_dt(kmb_i2s, kmb_i2s_dai);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_snd_soc_register_component(dev, &kmb_component,
+					      intel_kmb_platform_dai,
+				ARRAY_SIZE(intel_kmb_platform_dai));
+	if (ret) {
+		dev_err(dev, "not able to register dai\n");
+		return ret;
+	}
+
+	if (kmb_i2s->master) {
+		kmb_i2s->clk_i2s = devm_clk_get(dev, "osc");
+		if (IS_ERR(kmb_i2s->clk_i2s)) {
+			dev_err(dev, "Failed to get osc clock\n");
+			return PTR_ERR(kmb_i2s->clk_i2s);
+		}
+
+		ret = clk_prepare_enable(kmb_i2s->clk_i2s);
+		if (ret < 0)
+			return ret;
+
+		ret = devm_add_action_or_reset(dev, kmb_disable_clk,
+					       kmb_i2s->clk_i2s);
+		if (ret) {
+			dev_err(dev, "Failed to add clk_i2s reset action\n");
+			return ret;
+		}
+	}
+
+	dev_set_drvdata(dev, kmb_i2s);
+
+	return ret;
+}
+
+static const struct of_device_id kmb_plat_of_match[] = {
+	{ .compatible = "intel,keembay-i2s", },
+	{}
+};
+
+static struct platform_driver kmb_plat_dai_driver = {
+	.driver		= {
+		.name		= "kmb-plat-dai",
+		.of_match_table = kmb_plat_of_match,
+	},
+	.probe		= kmb_plat_dai_probe,
+};
+
+module_platform_driver(kmb_plat_dai_driver);
+
+MODULE_DESCRIPTION("ASoC Intel KeemBay Platform driver");
+MODULE_AUTHOR("Sia Jee Heng <jee.heng.sia@intel.com>");
+MODULE_AUTHOR("Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:kmb_platform");
diff --git a/sound/soc/intel/keembay/kmb_platform.h b/sound/soc/intel/keembay/kmb_platform.h
new file mode 100644
index 0000000..2960065
--- /dev/null
+++ b/sound/soc/intel/keembay/kmb_platform.h
@@ -0,0 +1,145 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ *  Intel KeemBay Platform driver
+ *
+ *  Copyright (C) 2020 Intel Corporation.
+ *
+ */
+
+#ifndef KMB_PLATFORM_H_
+#define KMB_PLATFORMP_H_
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/types.h>
+
+/* Register values with reference to KMB databook v1.1 */
+/* common register for all channel */
+#define IER		0x000
+#define IRER		0x004
+#define ITER		0x008
+#define CER		0x00C
+#define CCR		0x010
+#define RXFFR		0x014
+#define TXFFR		0x018
+
+/* Interrupt status register fields */
+#define ISR_TXFO	BIT(5)
+#define ISR_TXFE	BIT(4)
+#define ISR_RXFO	BIT(1)
+#define ISR_RXDA	BIT(0)
+
+/* I2S Tx Rx Registers for all channels */
+#define LRBR_LTHR(x)	(0x40 * (x) + 0x020)
+#define RRBR_RTHR(x)	(0x40 * (x) + 0x024)
+#define RER(x)		(0x40 * (x) + 0x028)
+#define TER(x)		(0x40 * (x) + 0x02C)
+#define RCR(x)		(0x40 * (x) + 0x030)
+#define TCR(x)		(0x40 * (x) + 0x034)
+#define ISR(x)		(0x40 * (x) + 0x038)
+#define IMR(x)		(0x40 * (x) + 0x03C)
+#define ROR(x)		(0x40 * (x) + 0x040)
+#define TOR(x)		(0x40 * (x) + 0x044)
+#define RFCR(x)		(0x40 * (x) + 0x048)
+#define TFCR(x)		(0x40 * (x) + 0x04C)
+#define RFF(x)		(0x40 * (x) + 0x050)
+#define TFF(x)		(0x40 * (x) + 0x054)
+
+/* I2S COMP Registers */
+#define I2S_COMP_PARAM_2	0x01F0
+#define I2S_COMP_PARAM_1	0x01F4
+#define I2S_COMP_VERSION	0x01F8
+#define I2S_COMP_TYPE		0x01FC
+
+/* PSS_GEN_CTRL_I2S_GEN_CFG_0 Registers */
+#define I2S_GEN_CFG_0		0x000
+#define PSS_CPR_RST_EN		0x010
+#define PSS_CPR_RST_SET		0x014
+#define PSS_CPR_CLK_CLR		0x000
+#define PSS_CPR_AUX_RST_EN	0x070
+
+#define MASTER_MODE		BIT(13)
+
+/* Interrupt Flag */
+#define TX_INT_FLAG		GENMASK(5, 4)
+#define RX_INT_FLAG		GENMASK(1, 0)
+/*
+ * Component parameter register fields - define the I2S block's
+ * configuration.
+ */
+#define	COMP1_TX_WORDSIZE_3(r)		FIELD_GET(GENMASK(27, 25), (r))
+#define	COMP1_TX_WORDSIZE_2(r)		FIELD_GET(GENMASK(24, 22), (r))
+#define	COMP1_TX_WORDSIZE_1(r)		FIELD_GET(GENMASK(21, 19), (r))
+#define	COMP1_TX_WORDSIZE_0(r)		FIELD_GET(GENMASK(18, 16), (r))
+#define	COMP1_RX_ENABLED(r)		FIELD_GET(BIT(6), (r))
+#define	COMP1_TX_ENABLED(r)		FIELD_GET(BIT(5), (r))
+#define	COMP1_MODE_EN(r)		FIELD_GET(BIT(4), (r))
+#define	COMP1_APB_DATA_WIDTH(r)		FIELD_GET(GENMASK(1, 0), (r))
+#define	COMP2_RX_WORDSIZE_3(r)		FIELD_GET(GENMASK(12, 10), (r))
+#define	COMP2_RX_WORDSIZE_2(r)		FIELD_GET(GENMASK(9, 7), (r))
+#define	COMP2_RX_WORDSIZE_1(r)		FIELD_GET(GENMASK(5, 3), (r))
+#define	COMP2_RX_WORDSIZE_0(r)		FIELD_GET(GENMASK(2, 0), (r))
+
+/* Add 1 to the below registers to indicate the actual size */
+#define	COMP1_TX_CHANNELS(r)	(FIELD_GET(GENMASK(10, 9), (r)) + 1)
+#define	COMP1_RX_CHANNELS(r)	(FIELD_GET(GENMASK(8, 7), (r)) + 1)
+#define	COMP1_FIFO_DEPTH(r)	(FIELD_GET(GENMASK(3, 2), (r)) + 1)
+
+/* Number of entries in WORDSIZE and DATA_WIDTH parameter registers */
+#define	COMP_MAX_WORDSIZE	8	/* 3 bits register width */
+
+#define MAX_CHANNEL_NUM		8
+#define MIN_CHANNEL_NUM		2
+
+#define TWO_CHANNEL_SUPPORT	2	/* up to 2.0 */
+#define FOUR_CHANNEL_SUPPORT	4	/* up to 3.1 */
+#define SIX_CHANNEL_SUPPORT	6	/* up to 5.1 */
+#define EIGHT_CHANNEL_SUPPORT	8	/* up to 7.1 */
+
+#define DWC_I2S_PLAY	BIT(0)
+#define DWC_I2S_RECORD	BIT(1)
+#define DW_I2S_SLAVE	BIT(2)
+#define DW_I2S_MASTER	BIT(3)
+
+#define I2S_RXDMA	0x01C0
+#define I2S_TXDMA	0x01C8
+
+/*
+ * struct i2s_clk_config_data - represent i2s clk configuration data
+ * @chan_nr: number of channel
+ * @data_width: number of bits per sample (8/16/24/32 bit)
+ * @sample_rate: sampling frequency (8Khz, 16Khz, 48Khz)
+ */
+struct i2s_clk_config_data {
+	int chan_nr;
+	u32 data_width;
+	u32 sample_rate;
+};
+
+struct kmb_i2s_info {
+	void __iomem *i2s_base;
+	void __iomem *pss_base;
+	struct clk *clk_i2s;
+	struct clk *clk_apb;
+	int active;
+	unsigned int capability;
+	unsigned int i2s_reg_comp1;
+	unsigned int i2s_reg_comp2;
+	struct device *dev;
+	u32 ccr;
+	u32 xfer_resolution;
+	u32 fifo_th;
+	bool master;
+
+	struct i2s_clk_config_data config;
+	int (*i2s_clk_cfg)(struct i2s_clk_config_data *config);
+
+	/* data related to PIO transfers */
+	bool use_pio;
+	struct snd_pcm_substream *tx_substream;
+	struct snd_pcm_substream *rx_substream;
+	unsigned int tx_ptr;
+	unsigned int rx_ptr;
+};
+
+#endif /* KMB_PLATFORM_H_ */
-- 
1.9.1


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

* [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver
  2020-05-18  2:17 [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Sia Jee Heng
  2020-05-18  2:17 ` [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver Sia Jee Heng
@ 2020-05-18  2:17 ` Sia Jee Heng
  2020-05-18 17:09   ` Mark Brown
  2020-05-18  2:17 ` [PATCH v2 3/4] ASoC: Intel: Add makefiles and kconfig changes for KeemBay Sia Jee Heng
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Sia Jee Heng @ 2020-05-18  2:17 UTC (permalink / raw)
  To: alsa-devel; +Cc: liam.r.girdwood, broonie, tiwai, pierre-louis.bossart

Add KeemBay machine driver which glues the tlv320aic3204 codec driver
and kmb_platform driver to form the asoc sound driver.

Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
Signed-off-by: Sia Jee Heng <jee.heng.sia@intel.com>
---
 sound/soc/intel/boards/kmb_tlv3204.c | 144 +++++++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)
 create mode 100644 sound/soc/intel/boards/kmb_tlv3204.c

diff --git a/sound/soc/intel/boards/kmb_tlv3204.c b/sound/soc/intel/boards/kmb_tlv3204.c
new file mode 100644
index 0000000..813c291
--- /dev/null
+++ b/sound/soc/intel/boards/kmb_tlv3204.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*  KeemBay ASOC Machine driver
+ *
+ *  Copyright (C) 2020 Intel Corporation.
+ *
+ */
+
+#include <linux/module.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/soc.h>
+#include "../../codecs/tlv320aic32x4.h"
+
+static unsigned int channels[] = {
+	2,
+};
+
+static struct snd_pcm_hw_constraint_list constraints_ch = {
+	.count	= ARRAY_SIZE(channels),
+	.list	= channels,
+};
+
+static unsigned int rates[] = {
+	16000,
+	48000,
+};
+
+static struct snd_pcm_hw_constraint_list constraints_rates = {
+	.count	= ARRAY_SIZE(rates),
+	.list	= rates,
+};
+
+static int kmb_mach_hw_params(struct snd_pcm_substream *substream,
+			      struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	int ret;
+	unsigned int sysclk;
+
+	/* As per codec datasheet Sysclk = 256 * fs */
+	sysclk = 12288000;
+
+	/* set the codec system clock */
+	ret = snd_soc_dai_set_sysclk(codec_dai, 1, sysclk, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		dev_err(rtd->dev, "snd_soc_dai_set_sysclk err = %d\n", ret);
+
+	return ret;
+}
+
+static int kmb_mach_dai_link_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *str_runtime;
+
+	str_runtime = substream->runtime;
+
+	snd_pcm_hw_constraint_list(str_runtime, 0,
+				   SNDRV_PCM_HW_PARAM_CHANNELS,
+				   &constraints_ch);
+
+	snd_pcm_hw_constraint_list(str_runtime, 0,
+				   SNDRV_PCM_HW_PARAM_RATE,
+				   &constraints_rates);
+
+	return 0;
+}
+
+static const struct snd_soc_ops kmb_mach_dai_link_ops = {
+	.startup = kmb_mach_dai_link_startup,
+	.hw_params = kmb_mach_hw_params,
+};
+
+static const struct snd_soc_dapm_widget aic32x4_dapm_widgets[] = {
+	SND_SOC_DAPM_MIC("External Mic", NULL),
+	SND_SOC_DAPM_HP("Headphone", NULL),
+};
+
+static const struct snd_soc_dapm_route aic32x4_dapm_routes[] = {
+	{"Headphone", NULL, "HPL"},
+	{"Headphone", NULL, "HPR"},
+	{"IN3_R", NULL, "External Mic"},
+	{"IN3_L", NULL, "External Mic"},
+};
+
+/* Linking platform to the codec-drivers  */
+SND_SOC_DAILINK_DEFS(link1,
+	DAILINK_COMP_ARRAY(COMP_CPU("20140000.i2s")),
+	DAILINK_COMP_ARRAY(COMP_CODEC("tlv320aic32x4.2-0018",
+					"tlv320aic32x4-hifi")),
+	DAILINK_COMP_ARRAY(COMP_PLATFORM("20140000.i2s")));
+
+/* kmb digital audio interface glue */
+static struct snd_soc_dai_link kmb_mach_dais[] = {
+	{
+		.name		= "tlv320aic32x4",
+		.stream_name	= "TLV320AIC32X4",
+		.ops = &kmb_mach_dai_link_ops,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+				SND_SOC_DAIFMT_NB_NF |
+				SND_SOC_DAIFMT_CBS_CFS,
+		SND_SOC_DAILINK_REG(link1),
+	},
+};
+
+/* kmb audio machine driver */
+static struct snd_soc_card kmb_mach = {
+	.name = "kmb_audio_card",
+	.dai_link = kmb_mach_dais,
+	.num_links = ARRAY_SIZE(kmb_mach_dais),
+	.dapm_routes = aic32x4_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(aic32x4_dapm_routes),
+	.dapm_widgets = aic32x4_dapm_widgets,
+	.num_dapm_widgets =  ARRAY_SIZE(aic32x4_dapm_widgets),
+	.fully_routed = true,
+};
+
+static int kmb_mach_audio_probe(struct platform_device *pdev)
+{
+	kmb_mach.dev = &pdev->dev;
+
+	return devm_snd_soc_register_card(&pdev->dev, &kmb_mach);
+}
+
+static const struct of_device_id kmb_mach_of_match[] = {
+	{ .compatible = "intel,kmb-snd-asoc", },
+	{}
+};
+
+static struct platform_driver kmb_mach_audio = {
+	.probe = kmb_mach_audio_probe,
+	.driver = {
+		.name = "kmb_tlv3204",
+		.of_match_table = kmb_mach_of_match,
+	},
+};
+
+module_platform_driver(kmb_mach_audio)
+
+/* Module information */
+MODULE_DESCRIPTION("Intel Audio tlv3204 machine driver for KeemBay");
+MODULE_AUTHOR("Sia Jee Heng <jee.heng.sia@intel.com>");
+MODULE_AUTHOR("Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:kmb_tlv3204");
-- 
1.9.1


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

* [PATCH v2 3/4] ASoC: Intel: Add makefiles and kconfig changes for KeemBay
  2020-05-18  2:17 [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Sia Jee Heng
  2020-05-18  2:17 ` [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver Sia Jee Heng
  2020-05-18  2:17 ` [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver Sia Jee Heng
@ 2020-05-18  2:17 ` Sia Jee Heng
  2020-05-18  2:17 ` [PATCH v2 4/4] dt-bindings: sound: Add documentation for KeemBay sound card and i2s Sia Jee Heng
  2020-05-18 16:38 ` [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Pierre-Louis Bossart
  4 siblings, 0 replies; 12+ messages in thread
From: Sia Jee Heng @ 2020-05-18  2:17 UTC (permalink / raw)
  To: alsa-devel; +Cc: liam.r.girdwood, broonie, tiwai, pierre-louis.bossart

Add makefile and kconfig changes for KeemBay tlv320aic3204
machine driver and kmb_platform driver.

Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
Signed-off-by: Sia Jee Heng <jee.heng.sia@intel.com>
---
 sound/soc/intel/Kconfig         |  7 +++++++
 sound/soc/intel/Makefile        |  1 +
 sound/soc/intel/boards/Kconfig  | 15 +++++++++++++++
 sound/soc/intel/boards/Makefile |  4 ++++
 4 files changed, 27 insertions(+)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index c8de0bb..bc93448 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -244,6 +244,13 @@ config SND_SOC_ACPI_INTEL_MATCH
 
 endif ## SND_SOC_INTEL_SST_TOPLEVEL || SND_SOC_SOF_INTEL_TOPLEVEL
 
+config SND_SOC_INTEL_KEEMBAY
+	tristate "Keembay Platforms"
+	depends on OF && (ARM64 || COMPILE_TEST)
+	depends on COMMON_CLK
+	help
+	  If you have a Intel Keembay platform then enable this option
+	  by saying Y or m.
 
 # ASoC codec drivers
 source "sound/soc/intel/boards/Kconfig"
diff --git a/sound/soc/intel/Makefile b/sound/soc/intel/Makefile
index 8160520..f5aa32b 100644
--- a/sound/soc/intel/Makefile
+++ b/sound/soc/intel/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_SND_SOC_INTEL_HASWELL) += haswell/
 obj-$(CONFIG_SND_SOC_INTEL_BAYTRAIL) += baytrail/
 obj-$(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM) += atom/
 obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += skylake/
+obj-$(CONFIG_SND_SOC_INTEL_KEEMBAY) += keembay/
 
 # Machine support
 obj-$(CONFIG_SND_SOC) += boards/
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 556c310..45f9fe5 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -549,3 +549,18 @@ endif
 
 
 endif ## SND_SOC_INTEL_MACH
+
+if SND_SOC_INTEL_KEEMBAY
+
+config SND_SOC_INTEL_KEEMBAY_TLV320AIC3204_MACH
+	tristate "Keembay with TLV320AIC3204 codec"
+	depends on ARM64 || COMPILE_TEST
+	depends on I2C
+	select SND_SOC_TLV320AIC32X4
+	select SND_SOC_TLV320AIC32X4_I2C
+	help
+	  This adds support for ASoC machine driver for Intel Keembay platforms
+	  with TLV320AIC3204 codec.
+	  Say Y if you have such a device.
+	  If unsure select "N".
+endif ## SND_SOC_INTEL_KEEMBAY
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 1ef6e60..7201d07 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -69,3 +69,7 @@ obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) += snd-soc-skl_nau88l25_ss
 obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH) += snd-soc-skl_hda_dsp.o
 obj-$(CONFIG_SND_SOC_INTEL_SOF_DA7219_MAX98373_MACH) += snd-soc-sof_da7219_max98373.o
 obj-$(CONFIG_SND_SOC_INTEL_SOUNDWIRE_SOF_MACH) += snd-soc-sof-sdw.o
+
+# Intel KeemBay Machine
+snd-soc-keembay_tlv3204-objs := kmb_tlv3204.o
+obj-$(CONFIG_SND_SOC_INTEL_KEEMBAY_TLV320AIC3204_MACH) += snd-soc-keembay_tlv3204.o
-- 
1.9.1


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

* [PATCH v2 4/4] dt-bindings: sound: Add documentation for KeemBay sound card and i2s
  2020-05-18  2:17 [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Sia Jee Heng
                   ` (2 preceding siblings ...)
  2020-05-18  2:17 ` [PATCH v2 3/4] ASoC: Intel: Add makefiles and kconfig changes for KeemBay Sia Jee Heng
@ 2020-05-18  2:17 ` Sia Jee Heng
  2020-05-18 16:38 ` [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Pierre-Louis Bossart
  4 siblings, 0 replies; 12+ messages in thread
From: Sia Jee Heng @ 2020-05-18  2:17 UTC (permalink / raw)
  To: alsa-devel; +Cc: liam.r.girdwood, broonie, tiwai, pierre-louis.bossart

Document Intel KeemBay sound card and i2s DT bindings.

Signed-off-by: Sia Jee Heng <jee.heng.sia@intel.com>
---
 .../bindings/sound/intel,keembay-i2s.yaml          | 57 ++++++++++++++++++++++
 .../bindings/sound/intel,keembay-sound-card.yaml   | 30 ++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/intel,keembay-sound-card.yaml

diff --git a/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml b/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml
new file mode 100644
index 0000000..031c343
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Intel Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/intel,keembay-i2s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel KeemBay I2S Device Tree Bindings
+
+maintainers:
+  - Sia, Jee Heng <jee.heng.sia@intel.com>
+
+description: |
+ Intel KeemBay I2S
+
+properties:
+  compatible:
+    enum:
+      - intel,keembay-i2s
+
+  reg:
+    items:
+      - description: Should contain registers location and length
+
+  reg-names:
+    items:
+      - const: i2s-regs
+      - const: i2s_gen_cfg
+      - const: i2s_gen_cfg_count
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Bus Clock
+      - description: Module Clock
+
+  clock-names:
+    items:
+      - const: osc
+      - const: apb_clk
+
+examples:
+  - |
+     #include <dt-bindings/interrupt-controller/arm-gic.h>
+     #include <dt-bindings/interrupt-controller/irq.h>
+     #define KEEM_BAY_PSS_AUX_I2S3
+     #define KEEM_BAY_PSS_I2S3
+     i2s@20140000 {
+         compatible = "intel,keembay-i2s";
+         reg = <0x0 0x20140000 0x0 0x200 0x0 0x202a00a4 0x0 0x4 0x0 0x202a00c0 0x0 0x4>;
+         reg-names = "i2s-regs", "i2s_gen_cfg", "i2s_gen_cfg_count";
+         interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+         clock-names = "osc", "apb_clk";
+         clocks = <&scmi_clk KEEM_BAY_PSS_AUX_I2S3>, <&scmi_clk KEEM_BAY_PSS_I2S3>;
+     };
diff --git a/Documentation/devicetree/bindings/sound/intel,keembay-sound-card.yaml b/Documentation/devicetree/bindings/sound/intel,keembay-sound-card.yaml
new file mode 100644
index 0000000..cca413a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/intel,keembay-sound-card.yaml
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Intel Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/intel,keembay-sound-card.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel KeemBay Sound Card
+
+maintainers:
+  - Sia, Jee Heng <jee.heng.sia@intel.com>
+
+description: |
+ Intel KeemBay Sound Card DT Binding
+
+properties:
+  compatible:
+    enum:
+      - intel,kmb-snd-asoc
+
+  intel,pcm-audio:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle of the i2s
+
+examples:
+ - |
+    sound {
+      compatible = "intel,kmb-snd-asoc";
+      intel,pcm-audio = <&i2s3>;
+    };
-- 
1.9.1


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

* Re: [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver
  2020-05-18  2:17 [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Sia Jee Heng
                   ` (3 preceding siblings ...)
  2020-05-18  2:17 ` [PATCH v2 4/4] dt-bindings: sound: Add documentation for KeemBay sound card and i2s Sia Jee Heng
@ 2020-05-18 16:38 ` Pierre-Louis Bossart
  4 siblings, 0 replies; 12+ messages in thread
From: Pierre-Louis Bossart @ 2020-05-18 16:38 UTC (permalink / raw)
  To: Sia Jee Heng, alsa-devel; +Cc: liam.r.girdwood, broonie, tiwai



On 5/17/20 9:17 PM, Sia Jee Heng wrote:
> The below series of patches support the KeemBay ASoC driver.
> It enabled the tlv320aic3204 machine driver and the platform driver initialize
> the i2s to capture and playback the pcm data on the ARM. The i2s is running
> in polling mode.
> 
> There is no DSP in the KeemBay SoC. Users are rely on the Gstreamer plugin
> to perform some Audio preprocessing.

This patch series matches what was reviewed internally at Intel by Andy 
Shevchenko, Cezary and I, so for patches 1..3:

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Note that my review is mostly high-level, I don't personally have any 
knowledge or detailed information on this IP and architecture.

> 
> Change History:
> v2:
> - Corrected I2S naming for DT binding.
>    
> v1:
> - Initial version.
> 
> Sia Jee Heng (4):
>    ASoC: Intel: Add KeemBay platform driver
>    ASoC: Intel: Boards: Add KeemBay machine driver
>    ASoC: Intel: Add makefiles and kconfig changes for KeemBay
>    dt-bindings: sound: Add documentation for KeemBay sound card and i2s
> 
>   .../bindings/sound/intel,keembay-i2s.yaml          |  57 ++
>   .../bindings/sound/intel,keembay-sound-card.yaml   |  30 +
>   sound/soc/intel/Kconfig                            |   7 +
>   sound/soc/intel/Makefile                           |   1 +
>   sound/soc/intel/boards/Kconfig                     |  15 +
>   sound/soc/intel/boards/Makefile                    |   4 +
>   sound/soc/intel/boards/kmb_tlv3204.c               | 144 ++++
>   sound/soc/intel/keembay/Makefile                   |   4 +
>   sound/soc/intel/keembay/kmb_platform.c             | 746 +++++++++++++++++++++
>   sound/soc/intel/keembay/kmb_platform.h             | 145 ++++
>   10 files changed, 1153 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml
>   create mode 100644 Documentation/devicetree/bindings/sound/intel,keembay-sound-card.yaml
>   create mode 100644 sound/soc/intel/boards/kmb_tlv3204.c
>   create mode 100644 sound/soc/intel/keembay/Makefile
>   create mode 100644 sound/soc/intel/keembay/kmb_platform.c
>   create mode 100644 sound/soc/intel/keembay/kmb_platform.h
> 

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

* Re: [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver
  2020-05-18  2:17 ` [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver Sia Jee Heng
@ 2020-05-18 17:06   ` Mark Brown
  2020-05-27 13:20     ` Sia, Jee Heng
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2020-05-18 17:06 UTC (permalink / raw)
  To: Sia Jee Heng; +Cc: liam.r.girdwood, alsa-devel, tiwai, pierre-louis.bossart

[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]

On Mon, May 18, 2020 at 10:17:19AM +0800, Sia Jee Heng wrote:

> +++ b/sound/soc/intel/keembay/kmb_platform.c
> @@ -0,0 +1,746 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  Intel KeemBay Platform driver
> + *
> + *  Copyright (C) 2020 Intel Corporation.

Please keep the entire header C++ style so things look more consistent.

> +static void pcm_operation(struct kmb_i2s_info *kmb_i2s, bool playback)

Please namespace this function, it looks like a core ALSA call.  It'd
probably be better to namespace a bunch of the other functions called
i2s_ as well.

> +static inline void i2s_irq_trigger(struct kmb_i2s_info *kmb_i2s, u32 stream,
> +				   int chan_nr, bool trigger)
> +{
> +	u32 i, irq;
> +	u32 flag;
> +
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		flag = TX_INT_FLAG;
> +	else
> +		flag = RX_INT_FLAG;
> +
> +	for (i = 0; i < chan_nr / 2; i++) {
> +		irq = readl(kmb_i2s->i2s_base + IMR(i));
> +		irq = trigger ? irq & ~flag : irq | flag;

Please write this as a normal if statement to improve legibility.

> +static int kmb_configure_dai_by_dt(struct kmb_i2s_info *kmb_i2s,
> +				   struct snd_soc_dai_driver *kmb_i2s_dai)
> +{
> +	u32 comp1 = readl(kmb_i2s->i2s_base + I2S_COMP_PARAM_1);
> +
> +	if (COMP1_TX_ENABLED(comp1))
> +		kmb_i2s->capability |= DWC_I2S_PLAY;
> +
> +	if (COMP1_RX_ENABLED(comp1))
> +		kmb_i2s->capability |= DWC_I2S_RECORD;
> +
> +	return kmb_configure_dai(kmb_i2s, kmb_i2s_dai,
> +		I2S_SAMPLE_RATES);
> +}

This isn't actually reading the DT?

> +static void kmb_disable_clk(void *data)
> +{
> +	clk_disable_unprepare(data);
> +}

This function doesn't seem to be adding anything?

> +	ret = of_property_read_string(dev->of_node, "mode", &i2s_mode);
> +	if (ret < 0) {
> +		dev_err(dev, "Couldn't find the entry\n");
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(kmb_i2s->dev, "Mode = %s", i2s_mode);
> +
> +	ret = match_string(i2s_mode_name, ARRAY_SIZE(i2s_mode_name), i2s_mode);

This property isn't defined in the DT binding and should be configured
by the machine driver through a set_fmt() operation anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver
  2020-05-18  2:17 ` [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver Sia Jee Heng
@ 2020-05-18 17:09   ` Mark Brown
  2020-05-27 13:11     ` Sia, Jee Heng
  2020-05-29  4:42     ` Sia, Jee Heng
  0 siblings, 2 replies; 12+ messages in thread
From: Mark Brown @ 2020-05-18 17:09 UTC (permalink / raw)
  To: Sia Jee Heng; +Cc: liam.r.girdwood, alsa-devel, tiwai, pierre-louis.bossart

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

On Mon, May 18, 2020 at 10:17:20AM +0800, Sia Jee Heng wrote:
> Add KeemBay machine driver which glues the tlv320aic3204 codec driver
> and kmb_platform driver to form the asoc sound driver.

Why do you need a custom machine driver for this, as this is a DT based
platform and I'm not seeing anything complex or unusual in the code you
should be able to use audio-graph-card (or simple-card but the former is
more modern and should be preferred for new systems).

If this is required the DT binding should be documented.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* RE: [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver
  2020-05-18 17:09   ` Mark Brown
@ 2020-05-27 13:11     ` Sia, Jee Heng
  2020-05-29  4:42     ` Sia, Jee Heng
  1 sibling, 0 replies; 12+ messages in thread
From: Sia, Jee Heng @ 2020-05-27 13:11 UTC (permalink / raw)
  To: Mark Brown; +Cc: liam.r.girdwood, alsa-devel, tiwai, pierre-louis.bossart

Thanks but will deep dive into the graph card. Will get back to you shortly.

Thanks
Regards
Jee Heng

-----Original Message-----
From: Mark Brown <broonie@kernel.org> 
Sent: Tuesday, May 19, 2020 1:10 AM
To: Sia, Jee Heng <jee.heng.sia@intel.com>
Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre-louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com
Subject: Re: [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver

On Mon, May 18, 2020 at 10:17:20AM +0800, Sia Jee Heng wrote:
> Add KeemBay machine driver which glues the tlv320aic3204 codec driver 
> and kmb_platform driver to form the asoc sound driver.

Why do you need a custom machine driver for this, as this is a DT based platform and I'm not seeing anything complex or unusual in the code you should be able to use audio-graph-card (or simple-card but the former is more modern and should be preferred for new systems).

If this is required the DT binding should be documented.

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

* RE: [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver
  2020-05-18 17:06   ` Mark Brown
@ 2020-05-27 13:20     ` Sia, Jee Heng
  2020-05-27 13:23       ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Sia, Jee Heng @ 2020-05-27 13:20 UTC (permalink / raw)
  To: Mark Brown; +Cc: liam.r.girdwood, alsa-devel, tiwai, pierre-louis.bossart



-----Original Message-----
From: Mark Brown <broonie@kernel.org> 
Sent: Tuesday, May 19, 2020 1:07 AM
To: Sia, Jee Heng <jee.heng.sia@intel.com>
Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre-louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com
Subject: Re: [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver

On Mon, May 18, 2020 at 10:17:19AM +0800, Sia Jee Heng wrote:

> +++ b/sound/soc/intel/keembay/kmb_platform.c
> @@ -0,0 +1,746 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  Intel KeemBay Platform driver
> + *
> + *  Copyright (C) 2020 Intel Corporation.

Please keep the entire header C++ style so things look more consistent.
[>>]  Will below format meet the C++ style?
[>>] // SPDX-License-Identifier: GPL-2.0-only
[>>] //  Copyright (C) 2020 Intel Corporation.
[>>] /*
[>>]   *  Intel KeemBay Platform driver
[>>]   */

> +static void pcm_operation(struct kmb_i2s_info *kmb_i2s, bool 
> +playback)

Please namespace this function, it looks like a core ALSA call.  It'd probably be better to namespace a bunch of the other functions called i2s_ as well.
[>>]  OK
> +static inline void i2s_irq_trigger(struct kmb_i2s_info *kmb_i2s, u32 stream,
> +				   int chan_nr, bool trigger)
> +{
> +	u32 i, irq;
> +	u32 flag;
> +
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		flag = TX_INT_FLAG;
> +	else
> +		flag = RX_INT_FLAG;
> +
> +	for (i = 0; i < chan_nr / 2; i++) {
> +		irq = readl(kmb_i2s->i2s_base + IMR(i));
> +		irq = trigger ? irq & ~flag : irq | flag;

Please write this as a normal if statement to improve legibility.
[>>]  OK
> +static int kmb_configure_dai_by_dt(struct kmb_i2s_info *kmb_i2s,
> +				   struct snd_soc_dai_driver *kmb_i2s_dai) {
> +	u32 comp1 = readl(kmb_i2s->i2s_base + I2S_COMP_PARAM_1);
> +
> +	if (COMP1_TX_ENABLED(comp1))
> +		kmb_i2s->capability |= DWC_I2S_PLAY;
> +
> +	if (COMP1_RX_ENABLED(comp1))
> +		kmb_i2s->capability |= DWC_I2S_RECORD;
> +
> +	return kmb_configure_dai(kmb_i2s, kmb_i2s_dai,
> +		I2S_SAMPLE_RATES);
> +}

This isn't actually reading the DT?

> +static void kmb_disable_clk(void *data) {
> +	clk_disable_unprepare(data);
> +}

This function doesn't seem to be adding anything?
[>>]  It intends to disable the clock during error return.
> +	ret = of_property_read_string(dev->of_node, "mode", &i2s_mode);
> +	if (ret < 0) {
> +		dev_err(dev, "Couldn't find the entry\n");
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(kmb_i2s->dev, "Mode = %s", i2s_mode);
> +
> +	ret = match_string(i2s_mode_name, ARRAY_SIZE(i2s_mode_name), 
> +i2s_mode);

This property isn't defined in the DT binding and should be configured by the machine driver through a set_fmt() operation anyway.

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

* Re: [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver
  2020-05-27 13:20     ` Sia, Jee Heng
@ 2020-05-27 13:23       ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2020-05-27 13:23 UTC (permalink / raw)
  To: Sia, Jee Heng; +Cc: liam.r.girdwood, alsa-devel, tiwai, pierre-louis.bossart

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Wed, May 27, 2020 at 01:20:41PM +0000, Sia, Jee Heng wrote:
> On Mon, May 18, 2020 at 10:17:19AM +0800, Sia Jee Heng wrote:

> > +++ b/sound/soc/intel/keembay/kmb_platform.c
> > @@ -0,0 +1,746 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + *  Intel KeemBay Platform driver
> > + *
> > + *  Copyright (C) 2020 Intel Corporation.

> Please keep the entire header C++ style so things look more consistent.
> [>>]  Will below format meet the C++ style?
> [>>] // SPDX-License-Identifier: GPL-2.0-only
> [>>] //  Copyright (C) 2020 Intel Corporation.
> [>>] /*
> [>>]   *  Intel KeemBay Platform driver
> [>>]   */

The *entire* comment please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* RE: [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver
  2020-05-18 17:09   ` Mark Brown
  2020-05-27 13:11     ` Sia, Jee Heng
@ 2020-05-29  4:42     ` Sia, Jee Heng
  1 sibling, 0 replies; 12+ messages in thread
From: Sia, Jee Heng @ 2020-05-29  4:42 UTC (permalink / raw)
  To: Mark Brown; +Cc: liam.r.girdwood, alsa-devel, tiwai, pierre-louis.bossart

Shall move on with the graph card and shall submit the changes.

-----Original Message-----
From: Mark Brown <broonie@kernel.org> 
Sent: Tuesday, May 19, 2020 1:10 AM
To: Sia, Jee Heng <jee.heng.sia@intel.com>
Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre-louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com
Subject: Re: [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver

On Mon, May 18, 2020 at 10:17:20AM +0800, Sia Jee Heng wrote:
> Add KeemBay machine driver which glues the tlv320aic3204 codec driver 
> and kmb_platform driver to form the asoc sound driver.

Why do you need a custom machine driver for this, as this is a DT based platform and I'm not seeing anything complex or unusual in the code you should be able to use audio-graph-card (or simple-card but the former is more modern and should be preferred for new systems).

If this is required the DT binding should be documented.

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

end of thread, other threads:[~2020-05-29  4:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18  2:17 [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Sia Jee Heng
2020-05-18  2:17 ` [PATCH v2 1/4] ASoC: Intel: Add KeemBay platform driver Sia Jee Heng
2020-05-18 17:06   ` Mark Brown
2020-05-27 13:20     ` Sia, Jee Heng
2020-05-27 13:23       ` Mark Brown
2020-05-18  2:17 ` [PATCH v2 2/4] ASoC: Intel: Boards: Add KeemBay machine driver Sia Jee Heng
2020-05-18 17:09   ` Mark Brown
2020-05-27 13:11     ` Sia, Jee Heng
2020-05-29  4:42     ` Sia, Jee Heng
2020-05-18  2:17 ` [PATCH v2 3/4] ASoC: Intel: Add makefiles and kconfig changes for KeemBay Sia Jee Heng
2020-05-18  2:17 ` [PATCH v2 4/4] dt-bindings: sound: Add documentation for KeemBay sound card and i2s Sia Jee Heng
2020-05-18 16:38 ` [PATCH v2 0/4] ASoC: Intel: Add KeemBay ASoC driver Pierre-Louis Bossart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.