Hi Morimoto-san,
On Wed, 9 Apr 2025 at 03:05, Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as
> both SPI and I2S. Adds MSIOF-I2S driver.
>
> MSIOF-SPI/I2S are using same DT compatible properties.
> MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2,
> MSIOF-SPI doesn't use Of-Graph.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
Thanks for your patch!
> --- a/sound/soc/renesas/Kconfig
> +++ b/sound/soc/renesas/Kconfig
> @@ -46,6 +46,13 @@ config SND_SOC_RCAR
> help
> This option enables R-Car SRU/SCU/SSIU/SSI sound support
>
> +config SND_SOC_MSIOF
> + tristate "R-Car series MSIOF support"
> + depends on OF
depends on ARCH_RENESAS || COMPILE_TEST
> + select SND_DMAENGINE_PCM
> + help
> + This option enables R-Car MSIOF sound support
> +
> config SND_SOC_RZ
> tristate "RZ/G2L series SSIF-2 support"
> depends on ARCH_RZG2L || COMPILE_TEST
> diff --git a/sound/soc/renesas/rcar/Makefile b/sound/soc/renesas/rcar/Makefile
> index 45eb875a912a..3a2c875595bd 100644
> --- a/sound/soc/renesas/rcar/Makefile
> +++ b/sound/soc/renesas/rcar/Makefile
> @@ -1,3 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> snd-soc-rcar-y := core.o gen.o dma.o adg.o ssi.o ssiu.o src.o ctu.o mix.o dvc.o cmd.o debugfs.o
> obj-$(CONFIG_SND_SOC_RCAR) += snd-soc-rcar.o
> +
> +snd-soc-msiof-y := msiof.o
> +obj-$(CONFIG_SND_SOC_MSIOF) += snd-soc-msiof.o
> diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c
> new file mode 100644
> index 000000000000..de8de3468402
> --- /dev/null
> +++ b/sound/soc/renesas/rcar/msiof.c
> @@ -0,0 +1,579 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Renesas R-Car MSIOF (Clock-Synchronized Serial Interface with FIFO) I2S driver
> +//
> +// Copyright (C) 2025 Renesas Solutions Corp.
> +// Author: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> +//
> +
> +/*
> + * [NOTE]
> + *
> + * This driver doesn't support Clock/Frame Provider Mode
> + *
> + * Basically MSIOF is created for SPI, but we can use it as I2S (Sound). Because of it, when we use
> + * it as I2S (Sound) with Provider Mode, we need to send dummy TX data even though it is used for
> + * RX. Because SPI HW needs TX Clock/Frame output for RX purpose also. It makes driver code complex.
> + *
> + * And when we use MSIOF (Sound) as Provider Mode, the clock source is [MSO clock] (= 133.33MHz)
> + * SoC internal clock. It is not for 48kHz/44.1kHz base clock. Thus the output/input will not be
> + * accurate sound.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <sound/dmaengine_pcm.h>
> +#include <sound/soc.h>
> +
> +/* register */
> +#define SITMDR1 0x00
> +#define SITMDR2 0x04
> +#define SITMDR3 0x08
> +#define SIRMDR1 0x10
> +#define SIRMDR2 0x14
> +#define SIRMDR3 0x18
> +#define SITSCR 0x20
> +#define SICTR 0x28
> +#define SIFCTR 0x30
> +#define SISTR 0x40
> +#define SIIER 0x44
> +#define SITFDR 0x50
> +#define SIRFDR 0x60
[...]
Perhaps the register definitions can be shared with spi-sh-msiof.c,
by extracting them into a shared header file?
Note that I have already converted drivers/spi/spi-sh-msiof.c locally
to use FIELD_PREP() (which requires changes to some macros), so we
may want to implement the sharing later.
> +static int msiof_hw_start(struct snd_soc_component *component,
> + struct snd_pcm_substream *substream, int cmd)
> +{
> + struct msiof_priv *priv = snd_soc_component_get_drvdata(component);
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + int is_play = msiof_is_play(substream);
> + int width = snd_pcm_format_width(runtime->format);
> + u32 val;
> +
> + /*
> + * see
> + * Datasheet 109.3.6 [Transmit and Receive Procedures]
> + *
> + * TX: Fig 109.14 - Fig 109.23
> + * RX: Fig 109.15
> + */
> +
> + /* reset errors */
> + priv->err_syc[substream->stream] =
> + priv->err_ovf[substream->stream] =
> + priv->err_udf[substream->stream] = 0;
> +
> + /* SITMDRx */
> + if (is_play) {
> + val = PCON | SYNCMD_LR | SYNCAC | TXSTP;
> + if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
> + val |= DTDL_1;
> +
> + msiof_write(priv, SITMDR1, val);
> +
> + val = BITLEN1(width);
> + msiof_write(priv, SITMDR2, val | GRP);
> + msiof_write(priv, SITMDR3, val);
> +
Don't you have to initialize SITMDR[123] unconditionally, as reception
requires transmitting dummy data on R-Car (cfr. SPI_CONTROLLER_MUST_TX)?
> + }
> + /* SIRMDRx */
> + else {
> + val = SYNCMD_LR | SYNCAC;
> + if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY))
> + val |= DTDL_1;
> +
> + msiof_write(priv, SIRMDR1, val);
> +
> + val = BITLEN1(width);
> + msiof_write(priv, SIRMDR2, val | GRP);
> + msiof_write(priv, SIRMDR3, val);
> + }
> +
> + /* SIIER */
> + if (is_play)
> + val = TDREQE | TDMAE | SISTR_ERR_TX;
> + else
> + val = RDREQE | RDMAE | SISTR_ERR_RX;
Likewise for transmit control flags.
> + msiof_update(priv, SIIER, val, val);
> +
> + /* SICTR */
> + if (is_play)
> + val = TXE | TEDG;
> + else
> + val = RXE | REDG;
Likewise,
> + msiof_update_and_wait(priv, SICTR, val, val, val);
> +
> + msiof_status_clear(priv);
> +
> + /* Start DMAC */
> + snd_dmaengine_pcm_trigger(substream, cmd);
> +
> + return 0;
> +}
> +
> +static int msiof_hw_stop(struct snd_soc_component *component,
> + struct snd_pcm_substream *substream, int cmd)
> +{
> + struct msiof_priv *priv = snd_soc_component_get_drvdata(component);
> + struct device *dev = component->dev;
> + int is_play = msiof_is_play(substream);
> + u32 val;
> +
> + /* SIIER */
> + if (is_play)
> + val = TDREQE | TDMAE | SISTR_ERR_TX;
> + else
> + val = RDREQE | RDMAE | SISTR_ERR_RX;
Likewise.
> + msiof_update(priv, SIIER, val, 0);
> +
> + /* Stop DMAC */
> + snd_dmaengine_pcm_trigger(substream, cmd);
> +
> + /* SICTR */
> + if (is_play)
> + val = TXE;
> + else
> + val = RXE;
Likewise.
> + msiof_update_and_wait(priv, SICTR, val, 0, 0);
> +
> + /* indicate error status if exist */
> + if (priv->err_syc[substream->stream] ||
> + priv->err_ovf[substream->stream] ||
> + priv->err_udf[substream->stream])
> + dev_warn(dev, "FSERR(%s) = %d, FOVF = %d, FUDF = %d\n",
> + snd_pcm_direction_name(substream->stream),
> + priv->err_syc[substream->stream],
> + priv->err_ovf[substream->stream],
> + priv->err_udf[substream->stream]);
> +
> + return 0;
> +}
> +static int msiof_probe(struct platform_device *pdev)
> +{
> + struct msiof_priv *priv;
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct device_node *port;
> + int irq, ret;
> +
> + /* Check MSIOF as Sound mode or SPI mode */
> + port = of_graph_get_next_port(dev->of_node, NULL);
> + if (!port)
> + return -ENODEV;
> + of_node_put(port);
Just wondering: don't you need to use port? Or is that handled
elsewhere, in common sound code?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
