diff mbox series

[v1,08/10] ASoC: loongson: Add I2S controller driver as platform device

Message ID 282dadefdaac7917fd681a6e84a5f0f07d0557bc.1725518229.git.zhoubinbin@loongson.cn (mailing list archive)
State Superseded
Headers show
Series ASoC: Some issues about loongson i2s | expand

Commit Message

Binbin Zhou Sept. 5, 2024, 7:07 a.m. UTC
The Loongson I2S controller exists not only in PCI form (LS7A bridge
chip), but also in platform device form (Loongson-2K1000 SoC).

This patch adds support for platform device I2S controller.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/Kconfig             |  12 +-
 sound/soc/loongson/Makefile            |   3 +
 sound/soc/loongson/loongson_i2s_plat.c | 186 +++++++++++++++++++++++++
 3 files changed, 200 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/loongson/loongson_i2s_plat.c

Comments

Mark Brown Sept. 5, 2024, 2:36 p.m. UTC | #1
On Thu, Sep 05, 2024 at 03:07:21PM +0800, Binbin Zhou wrote:
> The Loongson I2S controller exists not only in PCI form (LS7A bridge
> chip), but also in platform device form (Loongson-2K1000 SoC).
> 
> This patch adds support for platform device I2S controller.

Can some of this be shared with the PCI version - is it the same IP in a
different wrapper, or is it a new IP?
kernel test robot Sept. 6, 2024, 1:08 a.m. UTC | #2
Hi Binbin,

kernel test robot noticed the following build errors:

[auto build test ERROR on 097a44db5747403b19d05a9664e8ec6adba27e3b]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/ASoC-dt-bindings-Add-Everest-ES8323-Codec/20240905-150958
base:   097a44db5747403b19d05a9664e8ec6adba27e3b
patch link:    https://lore.kernel.org/r/282dadefdaac7917fd681a6e84a5f0f07d0557bc.1725518229.git.zhoubinbin%40loongson.cn
patch subject: [PATCH v1 08/10] ASoC: loongson: Add I2S controller driver as platform device
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240906/202409060840.Rm0gPgE4-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240906/202409060840.Rm0gPgE4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409060840.Rm0gPgE4-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> sound/soc/loongson/loongson_i2s_pci.c:157:1: warning: data definition has no type or storage class
     157 | module_pci_driver(loongson_i2s_driver);
         | ^~~~~~~~~~~~~~~~~
>> sound/soc/loongson/loongson_i2s_pci.c:157:1: error: type defaults to 'int' in declaration of 'module_pci_driver' [-Wimplicit-int]
>> sound/soc/loongson/loongson_i2s_pci.c:157:1: error: parameter names (without types) in function declaration [-Wdeclaration-missing-parameter-type]
>> sound/soc/loongson/loongson_i2s_pci.c:149:26: warning: 'loongson_i2s_driver' defined but not used [-Wunused-variable]
     149 | static struct pci_driver loongson_i2s_driver = {
         |                          ^~~~~~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for SND_SOC_LOONGSON_I2S_PCI
   Depends on [n]: SOUND [=m] && SND [=m] && SND_SOC [=m] && (LOONGARCH || COMPILE_TEST [=y]) && PCI [=n]
   Selected by [m]:
   - SND_SOC_LOONGSON_CARD [=m] && SOUND [=m] && SND [=m] && SND_SOC [=m] && (LOONGARCH || COMPILE_TEST [=y])


vim +/loongson_i2s_driver +149 sound/soc/loongson/loongson_i2s_pci.c

d84881e06836dc1 Yingkun Meng 2023-06-15  148  
d84881e06836dc1 Yingkun Meng 2023-06-15 @149  static struct pci_driver loongson_i2s_driver = {
d84881e06836dc1 Yingkun Meng 2023-06-15  150  	.name = "loongson-i2s-pci",
d84881e06836dc1 Yingkun Meng 2023-06-15  151  	.id_table = loongson_i2s_ids,
d84881e06836dc1 Yingkun Meng 2023-06-15  152  	.probe = loongson_i2s_pci_probe,
d84881e06836dc1 Yingkun Meng 2023-06-15  153  	.driver = {
d84881e06836dc1 Yingkun Meng 2023-06-15  154  		.pm = pm_sleep_ptr(&loongson_i2s_pm),
d84881e06836dc1 Yingkun Meng 2023-06-15  155  	},
d84881e06836dc1 Yingkun Meng 2023-06-15  156  };
d84881e06836dc1 Yingkun Meng 2023-06-15 @157  module_pci_driver(loongson_i2s_driver);
d84881e06836dc1 Yingkun Meng 2023-06-15  158
kernel test robot Sept. 6, 2024, 6:18 a.m. UTC | #3
Hi Binbin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 097a44db5747403b19d05a9664e8ec6adba27e3b]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/ASoC-dt-bindings-Add-Everest-ES8323-Codec/20240905-150958
base:   097a44db5747403b19d05a9664e8ec6adba27e3b
patch link:    https://lore.kernel.org/r/282dadefdaac7917fd681a6e84a5f0f07d0557bc.1725518229.git.zhoubinbin%40loongson.cn
patch subject: [PATCH v1 08/10] ASoC: loongson: Add I2S controller driver as platform device
config: i386-kismet-CONFIG_SND_SOC_LOONGSON_I2S_PCI-CONFIG_SND_SOC_LOONGSON_CARD-0-0 (https://download.01.org/0day-ci/archive/20240906/202409061419.RBYUF8ou-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20240906/202409061419.RBYUF8ou-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409061419.RBYUF8ou-lkp@intel.com/

kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for SND_SOC_LOONGSON_I2S_PCI when selected by SND_SOC_LOONGSON_CARD
   WARNING: unmet direct dependencies detected for SND_SOC_LOONGSON_I2S_PCI
     Depends on [n]: SOUND [=y] && SND [=y] && SND_SOC [=y] && (LOONGARCH || COMPILE_TEST [=y]) && PCI [=n]
     Selected by [y]:
     - SND_SOC_LOONGSON_CARD [=y] && SOUND [=y] && SND [=y] && SND_SOC [=y] && (LOONGARCH || COMPILE_TEST [=y])
Geert Uytterhoeven Sept. 6, 2024, 11:37 a.m. UTC | #4
Hi Binbin,

On Thu, Sep 5, 2024 at 9:07 AM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> The Loongson I2S controller exists not only in PCI form (LS7A bridge
> chip), but also in platform device form (Loongson-2K1000 SoC).
>
> This patch adds support for platform device I2S controller.
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

Thanks for your patch!

> --- a/sound/soc/loongson/Kconfig
> +++ b/sound/soc/loongson/Kconfig
> @@ -13,10 +13,20 @@ config SND_SOC_LOONGSON_I2S_PCI
>           The controller is found in loongson bridge chips or SoCs,
>           and work as a PCI device.
>
> +config SND_SOC_LOONGSON_I2S_PLATFORM
> +       tristate "Loongson I2S controller as platform device"

depends on LOONGARCH || COMPILE_TEST

> +       select SND_SOC_GENERIC_DMAENGINE_PCM
> +       help
> +         Say Y or M if you want to add support for I2S driver for
> +         Loongson I2S controller.
> +
> +         The controller work as a platform device, found in loongson
> +         SoCs.
> +
>  config SND_SOC_LOONGSON_CARD
>         tristate "Loongson Sound Card Driver"
>         select SND_SOC_LOONGSON_I2S_PCI
> -       depends on PCI

"select SND_SOC_LOONGSON_I2S_PCI if PCI"?

> +       select SND_SOC_LOONGSON_I2S_PLATFORM

Or perhaps do it the other way around, i,e. let
SND_SOC_LOONGSON_I2S_{PCI,PLATFORM} select SND_SOC_LOONGSON_CARD?
That would be similar to SPI_LOONGSON_{PCI,PLATFORM}, which select
SPI_LOONGSON_CORE.

>         help
>           Say Y or M if you want to add support for SoC audio using
>           loongson I2S controller.

Gr{oetje,eeting}s,

                        Geert
Binbin Zhou Sept. 7, 2024, 8:08 a.m. UTC | #5
On Thu, Sep 5, 2024 at 8:36 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Sep 05, 2024 at 03:07:21PM +0800, Binbin Zhou wrote:
> > The Loongson I2S controller exists not only in PCI form (LS7A bridge
> > chip), but also in platform device form (Loongson-2K1000 SoC).
> >
> > This patch adds support for platform device I2S controller.
>
> Can some of this be shared with the PCI version - is it the same IP in a
> different wrapper, or is it a new IP?

Hi Mark:

Thanks for your reply.

To be exact, they are similar, such as the definition of the
controller registers.
But for example, DMA data processing is different. In the pci version
of i2s, the DMA controller is built-in, while the DMA controller here
is external, using ls2x-apbdma (drivers/dma/ls2x-apb-dma.c)

Thanks.
Binbin
Binbin Zhou Sept. 7, 2024, 8:18 a.m. UTC | #6
Hi Geert:

Thanks for your reply.

On Fri, Sep 6, 2024 at 5:37 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Binbin,
>
> On Thu, Sep 5, 2024 at 9:07 AM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> > The Loongson I2S controller exists not only in PCI form (LS7A bridge
> > chip), but also in platform device form (Loongson-2K1000 SoC).
> >
> > This patch adds support for platform device I2S controller.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>
> Thanks for your patch!
>
> > --- a/sound/soc/loongson/Kconfig
> > +++ b/sound/soc/loongson/Kconfig
> > @@ -13,10 +13,20 @@ config SND_SOC_LOONGSON_I2S_PCI
> >           The controller is found in loongson bridge chips or SoCs,
> >           and work as a PCI device.
> >
> > +config SND_SOC_LOONGSON_I2S_PLATFORM
> > +       tristate "Loongson I2S controller as platform device"
>
> depends on LOONGARCH || COMPILE_TEST

This is will put under SND_SOC_LOONGSON_CARD.
>
> > +       select SND_SOC_GENERIC_DMAENGINE_PCM
> > +       help
> > +         Say Y or M if you want to add support for I2S driver for
> > +         Loongson I2S controller.
> > +
> > +         The controller work as a platform device, found in loongson
> > +         SoCs.
> > +
> >  config SND_SOC_LOONGSON_CARD
> >         tristate "Loongson Sound Card Driver"
> >         select SND_SOC_LOONGSON_I2S_PCI
> > -       depends on PCI
>
> "select SND_SOC_LOONGSON_I2S_PCI if PCI"?
>
> > +       select SND_SOC_LOONGSON_I2S_PLATFORM
>
> Or perhaps do it the other way around, i,e. let
> SND_SOC_LOONGSON_I2S_{PCI,PLATFORM} select SND_SOC_LOONGSON_CARD?
> That would be similar to SPI_LOONGSON_{PCI,PLATFORM}, which select
> SPI_LOONGSON_CORE.

Yes, it would be clearer. I'll modify it in the next version.

Thanks.
Binbin
>
> >         help
> >           Say Y or M if you want to add support for SoC audio using
> >           loongson I2S controller.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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
Mark Brown Sept. 7, 2024, 12:14 p.m. UTC | #7
On Sat, Sep 07, 2024 at 02:08:08PM +0600, Binbin Zhou wrote:
> On Thu, Sep 5, 2024 at 8:36 PM Mark Brown <broonie@kernel.org> wrote:

> > Can some of this be shared with the PCI version - is it the same IP in a
> > different wrapper, or is it a new IP?

> To be exact, they are similar, such as the definition of the
> controller registers.
> But for example, DMA data processing is different. In the pci version
> of i2s, the DMA controller is built-in, while the DMA controller here
> is external, using ls2x-apbdma (drivers/dma/ls2x-apb-dma.c)

OK, if all the registers are the same it might be worth trying to share
that code but possibly not with the DMA being split like this.
diff mbox series

Patch

diff --git a/sound/soc/loongson/Kconfig b/sound/soc/loongson/Kconfig
index b8d7e2bade24..cd298f8b54cd 100644
--- a/sound/soc/loongson/Kconfig
+++ b/sound/soc/loongson/Kconfig
@@ -13,10 +13,20 @@  config SND_SOC_LOONGSON_I2S_PCI
 	  The controller is found in loongson bridge chips or SoCs,
 	  and work as a PCI device.
 
+config SND_SOC_LOONGSON_I2S_PLATFORM
+	tristate "Loongson I2S controller as platform device"
+	select SND_SOC_GENERIC_DMAENGINE_PCM
+	help
+	  Say Y or M if you want to add support for I2S driver for
+	  Loongson I2S controller.
+
+	  The controller work as a platform device, found in loongson
+	  SoCs.
+
 config SND_SOC_LOONGSON_CARD
 	tristate "Loongson Sound Card Driver"
 	select SND_SOC_LOONGSON_I2S_PCI
-	depends on PCI
+	select SND_SOC_LOONGSON_I2S_PLATFORM
 	help
 	  Say Y or M if you want to add support for SoC audio using
 	  loongson I2S controller.
diff --git a/sound/soc/loongson/Makefile b/sound/soc/loongson/Makefile
index 578030ad6563..f396259244a3 100644
--- a/sound/soc/loongson/Makefile
+++ b/sound/soc/loongson/Makefile
@@ -3,6 +3,9 @@ 
 snd-soc-loongson-i2s-pci-y := loongson_i2s_pci.o loongson_i2s.o loongson_dma.o
 obj-$(CONFIG_SND_SOC_LOONGSON_I2S_PCI) += snd-soc-loongson-i2s-pci.o
 
+snd-soc-loongson-i2s-plat-y := loongson_i2s_plat.o loongson_i2s.o
+obj-$(CONFIG_SND_SOC_LOONGSON_I2S_PLATFORM) += snd-soc-loongson-i2s-plat.o
+
 #Machine Support
 snd-soc-loongson-card-y := loongson_card.o
 obj-$(CONFIG_SND_SOC_LOONGSON_CARD) += snd-soc-loongson-card.o
diff --git a/sound/soc/loongson/loongson_i2s_plat.c b/sound/soc/loongson/loongson_i2s_plat.c
new file mode 100644
index 000000000000..668067753b1c
--- /dev/null
+++ b/sound/soc/loongson/loongson_i2s_plat.c
@@ -0,0 +1,186 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loongson I2S controller master mode dirver(platform device)
+ *
+ * Copyright (C) 2023-2024 Loongson Technology Corporation Limited
+ *
+ * Author: Yingkun Meng <mengyingkun@loongson.cn>
+ *         Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include "loongson_i2s.h"
+
+#define LOONGSON_I2S_RX_DMA_OFFSET	21
+#define LOONGSON_I2S_TX_DMA_OFFSET	18
+
+#define LOONGSON_DMA0_CONF	0x0
+#define LOONGSON_DMA1_CONF	0x1
+#define LOONGSON_DMA2_CONF	0x2
+#define LOONGSON_DMA3_CONF	0x3
+#define LOONGSON_DMA4_CONF	0x4
+
+/* periods_max = PAGE_SIZE / sizeof(struct ls_dma_chan_reg) */
+static const struct snd_pcm_hardware loongson_pcm_hardware = {
+	.info = SNDRV_PCM_INFO_MMAP |
+		SNDRV_PCM_INFO_INTERLEAVED |
+		SNDRV_PCM_INFO_MMAP_VALID |
+		SNDRV_PCM_INFO_RESUME |
+		SNDRV_PCM_INFO_PAUSE,
+	.formats = SNDRV_PCM_FMTBIT_S16_LE |
+		   SNDRV_PCM_FMTBIT_S20_3LE |
+		   SNDRV_PCM_FMTBIT_S24_LE,
+	.period_bytes_min = 128,
+	.period_bytes_max = 128 * 1024,
+	.periods_min = 1,
+	.periods_max = 64,
+	.buffer_bytes_max = 1024 * 1024,
+};
+
+static const struct snd_dmaengine_pcm_config loongson_dmaengine_pcm_config = {
+	.pcm_hardware = &loongson_pcm_hardware,
+	.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
+	.prealloc_buffer_size = 128 * 1024,
+};
+
+static int loongson_pcm_open(struct snd_soc_component *component,
+			     struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	if (substream->pcm->device & 1) {
+		runtime->hw.info &= ~SNDRV_PCM_INFO_INTERLEAVED;
+		runtime->hw.info |= SNDRV_PCM_INFO_NONINTERLEAVED;
+	}
+
+	if (substream->pcm->device & 2)
+		runtime->hw.info &= ~(SNDRV_PCM_INFO_MMAP |
+				      SNDRV_PCM_INFO_MMAP_VALID);
+	/*
+	 * For mysterious reasons (and despite what the manual says)
+	 * playback samples are lost if the DMA count is not a multiple
+	 * of the DMA burst size.  Let's add a rule to enforce that.
+	 */
+	snd_pcm_hw_constraint_step(runtime, 0,
+				   SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
+	snd_pcm_hw_constraint_step(runtime, 0,
+				   SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 128);
+	snd_pcm_hw_constraint_integer(substream->runtime,
+				      SNDRV_PCM_HW_PARAM_PERIODS);
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver loongson_i2s_component_driver = {
+	.name   = LS_I2S_DRVNAME,
+	.open	= loongson_pcm_open,
+};
+
+static const struct regmap_config loongson_i2s_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x14,
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int loongson_i2s_apbdma_config(struct platform_device *pdev)
+{
+	int val;
+	void __iomem *regs;
+
+	regs = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	val = readl(regs);
+	val |= LOONGSON_DMA2_CONF << LOONGSON_I2S_TX_DMA_OFFSET;
+	val |= LOONGSON_DMA3_CONF << LOONGSON_I2S_RX_DMA_OFFSET;
+	writel(val, regs);
+
+	return 0;
+}
+
+static int loongson_i2s_plat_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct loongson_i2s *i2s;
+	struct resource *res;
+	struct clk *i2s_clk;
+	int ret;
+
+	i2s = devm_kzalloc(dev, sizeof(*i2s), GFP_KERNEL);
+	if (!i2s)
+		return -ENOMEM;
+
+	ret = loongson_i2s_apbdma_config(pdev);
+	if (ret)
+		return ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	i2s->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(i2s->reg_base))
+		return dev_err_probe(dev, PTR_ERR(i2s->reg_base),
+				     "devm_ioremap_resource failed\n");
+
+	i2s->regmap = devm_regmap_init_mmio(dev, i2s->reg_base,
+					    &loongson_i2s_regmap_config);
+	if (IS_ERR(i2s->regmap))
+		return dev_err_probe(dev, PTR_ERR(i2s->regmap),
+				     "devm_regmap_init_mmio failed\n");
+
+	i2s->playback_dma_data.addr = res->start + LS_I2S_TX_DATA;
+	i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	i2s->playback_dma_data.maxburst = 4;
+
+	i2s->capture_dma_data.addr = res->start + LS_I2S_RX_DATA;
+	i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	i2s->capture_dma_data.maxburst = 4;
+
+	i2s_clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(i2s_clk))
+		return dev_err_probe(dev, PTR_ERR(i2s_clk), "clock property invalid\n");
+	i2s->clk_rate = clk_get_rate(i2s_clk);
+
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	dev_set_name(dev, LS_I2S_DRVNAME);
+	dev_set_drvdata(dev, i2s);
+
+	ret = devm_snd_soc_register_component(dev, &loongson_i2s_component_driver,
+					      &loongson_i2s_dai, 1);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register DAI\n");
+
+	return devm_snd_dmaengine_pcm_register(dev, &loongson_dmaengine_pcm_config,
+					       SND_DMAENGINE_PCM_FLAG_COMPAT);
+}
+
+static const struct of_device_id loongson_i2s_ids[] = {
+	{ .compatible = "loongson,ls2k1000-i2s" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, loongson_i2s_ids);
+
+static struct platform_driver loongson_i2s_driver = {
+	.probe = loongson_i2s_plat_probe,
+	.driver = {
+		.name = "loongson-i2s-plat",
+		.pm = pm_sleep_ptr(&loongson_i2s_pm),
+		.of_match_table = loongson_i2s_ids,
+	},
+};
+module_platform_driver(loongson_i2s_driver);
+
+MODULE_DESCRIPTION("Loongson I2S Master Mode ASoC Driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
+MODULE_LICENSE("GPL");