[PATCH 0/7] Add FFT Support for R-Car Gen3 devices

STALE1986d

47 messages, 7 authors, 2021-03-03 · open the first message on its own page

[PATCH 0/7] Add FFT Support for R-Car Gen3 devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:53:52

The DAB hardware accelerator found on R-Car E3 (a.k.a. r8a77990)
and R-Car M3-N (a.k.a. r8a77965) devices is a hardware accelerator
for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one
decoder module, compatible with DAB specification (ETSI EN 300 401
and ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing from de-puncture to final decoded result.

This series adds FFT support only for R-Car E3 and R-Car M3-N,
FIC and MSC support will be added later on.

Thanks,
Fab

Fabrizio Castro (7):
  clk: renesas: r8a77990: Add DAB clock
  clk: renesas: r8a77965: Add DAB clock
  dt-bindings: misc: Add binding for R-Car DAB
  misc: Add driver for DAB IP found on Renesas R-Car devices
  arm64: dts: renesas: r8a77990: Add DAB support
  arm64: dts: renesas: r8a77965: Add DAB support
  arm64: configs: Add R-Car DAB support

 .../devicetree/bindings/misc/renesas,dab.yaml |  75 ++++++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/renesas/r8a77965.dtsi     |  12 ++
 arch/arm64/boot/dts/renesas/r8a77990.dtsi     |  12 ++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/clk/renesas/r8a77965-cpg-mssr.c       |   1 +
 drivers/clk/renesas/r8a77990-cpg-mssr.c       |   1 +
 drivers/misc/Kconfig                          |   1 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/rcar_dab/Kconfig                 |  11 ++
 drivers/misc/rcar_dab/Makefile                |   8 +
 drivers/misc/rcar_dab/rcar_dev.c              | 176 ++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h              | 116 ++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c              | 160 ++++++++++++++++
 include/uapi/linux/rcar_dab.h                 |  35 ++++
 15 files changed, 617 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/renesas,dab.yaml
 create mode 100644 drivers/misc/rcar_dab/Kconfig
 create mode 100644 drivers/misc/rcar_dab/Makefile
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.c
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.h
 create mode 100644 drivers/misc/rcar_dab/rcar_fft.c
 create mode 100644 include/uapi/linux/rcar_dab.h

-- 
2.25.1

[PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:54:11

This patch adds the DAB clock to the R8A77990 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 drivers/clk/renesas/r8a77990-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/drivers/clk/renesas/r8a77990-cpg-mssr.c b/drivers/clk/renesas/r8a77990-cpg-mssr.c
index 2d172f80b34c..a582f2ec3294 100644
--- a/drivers/clk/renesas/r8a77990-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77990-cpg-mssr.c
@@ -232,6 +232,7 @@ static const struct mssr_mod_clk r8a77990_mod_clks[] __initconst = {
 	DEF_MOD("ssi2",			1013,	MOD_CLK_ID(1005)),
 	DEF_MOD("ssi1",			1014,	MOD_CLK_ID(1005)),
 	DEF_MOD("ssi0",			1015,	MOD_CLK_ID(1005)),
+	DEF_MOD("dab",			1016,	R8A77990_CLK_S3D1),
 	DEF_MOD("scu-all",		1017,	R8A77990_CLK_S3D4),
 	DEF_MOD("scu-dvc1",		1018,	MOD_CLK_ID(1017)),
 	DEF_MOD("scu-dvc0",		1019,	MOD_CLK_ID(1017)),
-- 
2.25.1

[PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:54:36

This patch adds the DAB clock to the R8A77965 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 drivers/clk/renesas/r8a77965-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/drivers/clk/renesas/r8a77965-cpg-mssr.c b/drivers/clk/renesas/r8a77965-cpg-mssr.c
index 46a157732759..bc1be8bcbbe4 100644
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -250,6 +250,7 @@ static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
 	DEF_MOD("ssi2",			1013,	MOD_CLK_ID(1005)),
 	DEF_MOD("ssi1",			1014,	MOD_CLK_ID(1005)),
 	DEF_MOD("ssi0",			1015,	MOD_CLK_ID(1005)),
+	DEF_MOD("dab",			1016,	R8A77965_CLK_S0D6),
 	DEF_MOD("scu-all",		1017,	R8A77965_CLK_S3D4),
 	DEF_MOD("scu-dvc1",		1018,	MOD_CLK_ID(1017)),
 	DEF_MOD("scu-dvc0",		1019,	MOD_CLK_ID(1017)),
-- 
2.25.1

[PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:54:36

Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 .../devicetree/bindings/misc/renesas,dab.yaml | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/renesas,dab.yaml
diff --git a/Documentation/devicetree/bindings/misc/renesas,dab.yaml b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
new file mode 100644
index 000000000000..e9494add13d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Renesas Electronics Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/renesas,dab.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car DAB Hardware Accelerator
+
+maintainers:
+  - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+
+description:
+  The DAB hardware accelerator found on some R-Car devices is a hardware
+  accelerator for software DAB demodulators.
+  It consists of one FFT (Fast Fourier Transform) module and one decoder module,
+  compatible with DAB specification (ETSI EN 300 401 and ETSI TS 102 563).
+  The decoder module can perform FIC decoding and MSC decoding processing from
+  de-puncture to final decoded result.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3 devices
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: dab
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  # R8A77990 (R-Car E3)
+  - |
+    #include <dt-bindings/clock/r8a77990-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a77990-sysc.h>
+
+    dab: dab@e6730000 {
+        compatible = "renesas,dab-r8a77990",
+                     "renesas,rcar-gen3-dab";
+        reg = <0xe6730000 0x120>;
+        interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD 1016>;
+        clock-names = "dab";
+        power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+        resets = <&cpg 1016>;
+        status = "disabled";
+    };
-- 
2.25.1

[PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:55:36

R-Car E3 (a.k.a. r8a77990) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 5010f23fafcc..5a6b835f137a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -963,6 +963,18 @@ ipmmu_vp0: iommu@fe990000 {
 			#iommu-cells = <1>;
 		};
 
+		dab: dab@e6730000 {
+			compatible = "renesas,dab-r8a77990",
+				     "renesas,rcar-gen3-dab";
+			reg = <0x00 0xe6730000 0x00 0x120>;
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1016>;
+			clock-names = "dab";
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 1016>;
+			status = "disabled";
+		};
+
 		avb: ethernet@e6800000 {
 			compatible = "renesas,etheravb-r8a77990",
 				     "renesas,etheravb-rcar-gen3";
-- 
2.25.1

[PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:55:57

The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176 +++++++++++++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
 9 files changed, 515 insertions(+)
 create mode 100644 drivers/misc/rcar_dab/Kconfig
 create mode 100644 drivers/misc/rcar_dab/Makefile
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.c
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.h
 create mode 100644 drivers/misc/rcar_dab/rcar_fft.c
 create mode 100644 include/uapi/linux/rcar_dab.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 16ada1a4b751..83cd7a42d11d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4917,6 +4917,13 @@ L:	linux-input@vger.kernel.org
 S:	Supported
 F:	drivers/input/keyboard/dlink-dir685-touchkeys.c
 
+DAB HW ACCELERATOR DRIVER FOR R-CAR
+M:	Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+L:	linux-renesas-soc@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/misc/renesas,dab.yaml
+F:	drivers/misc/rcar_dab/*
+
 DALLAS/MAXIM DS1685-FAMILY REAL TIME CLOCK
 M:	Joshua Kinard <kumba@gentoo.org>
 S:	Maintained
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f532c59bb59b..fe2ffb41e006 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -461,4 +461,5 @@ source "drivers/misc/bcm-vk/Kconfig"
 source "drivers/misc/cardreader/Kconfig"
 source "drivers/misc/habanalabs/Kconfig"
 source "drivers/misc/uacce/Kconfig"
+source "drivers/misc/rcar_dab/Kconfig"
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 99b6f15a3c70..cb8821383287 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
 obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
+obj-$(CONFIG_RCAR_DAB)		+= rcar_dab/
diff --git a/drivers/misc/rcar_dab/Kconfig b/drivers/misc/rcar_dab/Kconfig
new file mode 100644
index 000000000000..ea0d2e1e9c2e
--- /dev/null
+++ b/drivers/misc/rcar_dab/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# R-Car DAB Hardware Accelerator
+#
+
+config RCAR_DAB
+	tristate "DAB accelerator for Renesas R-Car devices"
+	depends on ARCH_R8A77990 || ARCH_R8A77965
+	help
+	  Some R-Car devices come with an IP to accelerate DAB decoding.
+	  Enable this option to add driver support.
diff --git a/drivers/misc/rcar_dab/Makefile b/drivers/misc/rcar_dab/Makefile
new file mode 100644
index 000000000000..2eb818bfa7bf
--- /dev/null
+++ b/drivers/misc/rcar_dab/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Makefile for the Renesas R-Car DAB driver
+#
+
+obj-$(CONFIG_RCAR_DAB) += rcar_dab.o
+
+rcar_dab-y := rcar_dev.o rcar_fft.o
diff --git a/drivers/misc/rcar_dab/rcar_dev.c b/drivers/misc/rcar_dab/rcar_dev.c
new file mode 100644
index 000000000000..f53da3037597
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_dev.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * R-Car Gen3 DAB hardware accelerator driver
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+
+#include <uapi/linux/rcar_dab.h>
+#include "rcar_dev.h"
+
+irqreturn_t rcar_dab_irq(int irq, void *devid)
+{
+	struct rcar_dab *dab = devid;
+	u32 intsr, intcr1;
+
+	spin_lock(&dab->shared_regs_lock);
+
+	intcr1 = rcar_dab_read(dab, RCAR_DAB_INTCR1);
+	rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x000003FF);
+
+	intsr = rcar_dab_read(dab, RCAR_DAB_INTSR);
+	if (!intsr) {
+		rcar_dab_write(dab, RCAR_DAB_INTCR1, intcr1);
+		spin_unlock(&dab->shared_regs_lock);
+		return IRQ_NONE;
+	}
+
+	/* Re-enable interrupts that haven't fired */
+	rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x3FF & (intsr | intcr1));
+	/* Clear interrupts */
+	rcar_dab_write(dab, RCAR_DAB_INTSR, 0x7 & ~intsr);
+
+	spin_unlock(&dab->shared_regs_lock);
+
+	if (intsr & RCAR_DAB_INTSR_FFT_DONE)
+		rcar_dab_fft_irq(dab);
+
+	return IRQ_HANDLED;
+}
+
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int cmd,
+				    unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	struct rcar_dab *dab;
+	int ret;
+
+	dab = container_of(file->private_data, struct rcar_dab, misc);
+
+	switch (cmd) {
+	case RCAR_DAB_IOC_FFT:
+		if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+			return -EFAULT;
+		ret = rcar_dab_fft(dab, argp);
+		break;
+	default:
+		ret = -ENOTTY;
+	}
+
+	return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= rcar_dab_unlocked_ioctl,
+};
+
+static int rcar_dab_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_dab *dab;
+	int ret, irq;
+
+	dab = devm_kzalloc(dev, sizeof(*dab), GFP_KERNEL);
+	if (!dab)
+		return -ENOMEM;
+	dab->pdev = pdev;
+
+	dab->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dab->base))
+		return PTR_ERR(dab->base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Can't get the irq.\n");
+		return -EINVAL;
+	}
+
+	dab->clk = devm_clk_get(&pdev->dev, "dab");
+	if (IS_ERR(dab->clk)) {
+		ret = PTR_ERR(dab->clk);
+		dev_err(dev, "Can't get the clock (%d).\n", ret);
+		return ret;
+	}
+
+	spin_lock_init(&dab->shared_regs_lock);
+
+	ret = clk_prepare_enable(dab->clk);
+	if (ret)
+		return ret;
+
+	ret = rcar_dab_fft_probe(dab);
+	if (ret)
+		goto error_clock_disable;
+
+	ret = devm_request_irq(dev, irq, rcar_dab_irq, 0, dev_name(dev), dab);
+	if (ret) {
+		dev_err(dev, "Can't request the irq (%d).\n", ret);
+		goto error_remove;
+	}
+
+	platform_set_drvdata(pdev, dab);
+
+	dab->misc.minor = MISC_DYNAMIC_MINOR;
+	dab->misc.name = RCAR_DAB_DRV_NAME;
+	dab->misc.fops = &rcar_dab_fops;
+	ret = misc_register(&dab->misc);
+	if (ret) {
+		dev_err(dev, "Can't register misc device (%d).\n", ret);
+		goto error_remove;
+	}
+
+	dev_info(dev, "R-Car Gen3 DAB misc driver ready.\n");
+
+	return 0;
+
+error_remove:
+	rcar_dab_fft_remove(dab);
+
+error_clock_disable:
+	clk_disable_unprepare(dab->clk);
+
+	return ret;
+}
+
+static int rcar_dab_remove(struct platform_device *pdev)
+{
+	struct rcar_dab *dab = platform_get_drvdata(pdev);
+
+	misc_deregister(&dab->misc);
+	rcar_dab_fft_remove(dab);
+	clk_disable_unprepare(dab->clk);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_dab_of_table[] = {
+	{ .compatible = "renesas,rcar-gen3-dab" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, rcar_dab_of_table);
+
+static struct platform_driver rcar_dab_driver = {
+	.driver			= {
+		.name		= RCAR_DAB_DRV_NAME,
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(rcar_dab_of_table),
+	},
+	.probe		= rcar_dab_probe,
+	.remove		= rcar_dab_remove,
+};
+
+module_platform_driver(rcar_dab_driver);
+
+MODULE_AUTHOR("Fabrizio Castro <fabrizio.castro.jz@renesas.com>");
+MODULE_DESCRIPTION("Driver for R-Car DAB IP");
+MODULE_LICENSE("GPL");
diff --git a/drivers/misc/rcar_dab/rcar_dev.h b/drivers/misc/rcar_dab/rcar_dev.h
new file mode 100644
index 000000000000..1fc6d1fc04d6
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_dev.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * R-Car Gen3 DAB driver shared definitions
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#ifndef _RCAR_DEV_H_
+#define _RCAR_DEV_H_
+
+#include <linux/io.h>
+#include <linux/miscdevice.h>
+
+#define RCAR_DAB_DRV_NAME			"rcar_dab"
+
+/* FFT registers offset */
+#define RCAR_DAB_FFTCR				0x000
+#define RCAR_DAB_FFTSSR				0x004
+#define RCAR_DAB_FFTREQCR			0x00C
+#define RCAR_DAB_FFTINADDR			0x010
+#define RCAR_DAB_FFTOUTADDR			0x014
+#define RCAR_DAB_FFTNUMOFDMR			0x018
+
+/* Interrupt registers offset */
+#define RCAR_DAB_INTSR				0x0C0
+#define RCAR_DAB_INTCR1				0x0C4
+
+/* Register DAB_FFTCR */
+#define RCAR_DAB_FFTCR_FFT_EN_DISABLED		0
+#define RCAR_DAB_FFTCR_FFT_EN_ENABLED		1
+
+/* Register DAB_FFTREQCR */
+#define RCAR_DAB_FFTREQCR_FFT_REQ_INACTIVE	0
+#define RCAR_DAB_FFTREQCR_FFT_REQ_ACTIVE	1
+
+/* Register DAB_INTSR */
+#define RCAR_DAB_INTSR_FFT_DONE			0x1
+
+/* Register DAB_INTCR1 */
+#define RCAR_DAB_INTCR1_FFT_DONE_MASK		0x1
+#define RCAR_DAB_INTCR1_FFT_DONE_INT_ENABLED	0
+#define RCAR_DAB_INTCR1_FFT_DONE_INT_DISABLED	1
+
+struct rcar_dab_fft {
+	bool done;			/*
+					 * Condition for waking up the process
+					 * sleeping while FFT is in progress.
+					 */
+	wait_queue_head_t wait;		/* Wait queue for FFT. */
+	struct mutex lock;		/* Mutex for FFT. */
+	dma_addr_t dma_input_buf;	/*
+					 * Input buffer seen by the FFT
+					 * module.
+					 */
+	dma_addr_t dma_output_buf;	/*
+					 * Output buffer seen by the FFT
+					 * module.
+					 */
+	void *input_buffer;		/* Input buffer seen by the CPU */
+	void *output_buffer;		/* Output buffer seen by the CPU */
+};
+
+struct rcar_dab {
+	void __iomem *base;		/* Base register address. */
+	struct clk *clk;		/* Module clock. */
+	struct platform_device *pdev;	/* Platform device. */
+	spinlock_t shared_regs_lock;	/*
+					 * Spin lock for accessing shared
+					 * registers.
+					 */
+	struct miscdevice misc;		/* misc structure */
+	struct rcar_dab_fft fft;	/* FFT specific data */
+};
+
+static inline void rcar_dab_write(struct rcar_dab *dab, u32 offset, u32 data)
+{
+	writel(data, dab->base + offset);
+}
+
+static inline u32 rcar_dab_read(struct rcar_dab *dab, u32 offset)
+{
+	return readl(dab->base + offset);
+}
+
+/*
+ * Most of the registers are module specific (either FFT, FIC, or MSC specific)
+ * and contain only one field. Furthermore, most of the registers are accessed
+ * in the context of single process, therefore we don't need to make sure we
+ * serialize access to those registers. However, some of the registers are
+ * shared among modules (like the interrupt specific ones), and the irq function
+ * might try and access those registers while a process is requesting some
+ * module specific processing, therefore accessing those registers require
+ * serialization via spin locks.
+ */
+static inline void rcar_dab_update_bits_locked(struct rcar_dab *dab,
+					       u32 offset, u32 mask, u32 val)
+{
+	u32 tmp, orig;
+
+	spin_lock(&dab->shared_regs_lock);
+
+	orig = rcar_dab_read(dab, offset);
+	tmp = orig & ~mask;
+	tmp |= val & mask;
+	rcar_dab_write(dab, offset, tmp);
+
+	spin_unlock(&dab->shared_regs_lock);
+}
+
+int rcar_dab_fft_probe(struct rcar_dab *dab);
+void rcar_dab_fft_remove(struct rcar_dab *dab);
+void rcar_dab_fft_irq(struct rcar_dab *dab);
+void rcar_dab_fft_disable(struct rcar_dab *dab);
+int rcar_dab_fft(struct rcar_dab *dab, struct rcar_dab_fft_req __user *fft_req);
+
+#endif /* _RCAR_DEV_H_ */
diff --git a/drivers/misc/rcar_dab/rcar_fft.c b/drivers/misc/rcar_dab/rcar_fft.c
new file mode 100644
index 000000000000..bc064dc4de4c
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_fft.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * R-Car Gen3 DAB FFT implementation
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+
+#include <uapi/linux/rcar_dab.h>
+#include "rcar_dev.h"
+
+#define FFT_MAX_BUFFER_SIZE		8192
+
+/* The index of the array corresponds to the value for FFT_MODE */
+static const unsigned int rcar_dab_fft_size_lut[] = {
+	256, 512, 1024, 2048
+};
+
+int rcar_dab_fft_probe(struct rcar_dab *dab)
+{
+	struct device *dev = &dab->pdev->dev;
+
+	rcar_dab_fft_disable(dab);
+	mutex_init(&dab->fft.lock);
+	init_waitqueue_head(&dab->fft.wait);
+
+	dab->fft.input_buffer = dma_alloc_coherent(dev, FFT_MAX_BUFFER_SIZE,
+						   &dab->fft.dma_input_buf,
+						   GFP_KERNEL);
+	if (!dab->fft.input_buffer)
+		return -ENOMEM;
+
+	dab->fft.output_buffer = dma_alloc_coherent(dev, FFT_MAX_BUFFER_SIZE,
+						    &dab->fft.dma_output_buf,
+						    GFP_KERNEL);
+	if (!dab->fft.output_buffer) {
+		dma_free_coherent(dev, FFT_MAX_BUFFER_SIZE,
+				  dab->fft.input_buffer,
+				  dab->fft.dma_input_buf);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void rcar_dab_fft_remove(struct rcar_dab *dab)
+{
+	struct device *dev = &dab->pdev->dev;
+
+	rcar_dab_fft_disable(dab);
+
+	dma_free_coherent(dev, FFT_MAX_BUFFER_SIZE, dab->fft.input_buffer,
+			  dab->fft.dma_input_buf);
+	dma_free_coherent(dev, FFT_MAX_BUFFER_SIZE, dab->fft.output_buffer,
+			  dab->fft.dma_output_buf);
+}
+
+static int rcar_dab_fft_init(struct rcar_dab *dab, struct rcar_dab_fft_req *fft)
+{
+	u32 mode;
+
+	for (mode = 0; mode < ARRAY_SIZE(rcar_dab_fft_size_lut); mode++)
+		if (rcar_dab_fft_size_lut[mode] == fft->points)
+			break;
+	if (mode == ARRAY_SIZE(rcar_dab_fft_size_lut))
+		return -EINVAL;
+	if (fft->ofdm_number == 0)
+		return -EINVAL;
+
+	rcar_dab_write(dab, RCAR_DAB_FFTSSR, mode);
+	rcar_dab_write(dab, RCAR_DAB_FFTNUMOFDMR, fft->ofdm_number);
+	rcar_dab_write(dab, RCAR_DAB_FFTINADDR, (u32)dab->fft.dma_input_buf);
+	rcar_dab_write(dab, RCAR_DAB_FFTOUTADDR, (u32)dab->fft.dma_output_buf);
+
+	return 0;
+}
+
+void rcar_dab_fft_disable(struct rcar_dab *dab)
+{
+	/* Disable FFT interrupt in case it's enabled */
+	rcar_dab_update_bits_locked(dab, RCAR_DAB_INTCR1,
+				    RCAR_DAB_INTCR1_FFT_DONE_MASK,
+				    RCAR_DAB_INTCR1_FFT_DONE_INT_DISABLED);
+
+	/* Clear FFT interrupt status */
+	rcar_dab_update_bits_locked(dab, RCAR_DAB_INTSR,
+				    RCAR_DAB_INTSR_FFT_DONE, 0);
+
+	/* Mark FFT req as inactive */
+	rcar_dab_write(dab, RCAR_DAB_FFTREQCR,
+		       RCAR_DAB_FFTREQCR_FFT_REQ_INACTIVE);
+
+	/* Disable FFT */
+	rcar_dab_write(dab, RCAR_DAB_FFTCR, RCAR_DAB_FFTCR_FFT_EN_DISABLED);
+}
+
+static void rcar_dab_fft_enable(struct rcar_dab *dab)
+{
+	/* Clear FFT interrupt status */
+	rcar_dab_update_bits_locked(dab, RCAR_DAB_INTSR,
+				    RCAR_DAB_INTSR_FFT_DONE, 0);
+
+	/* Enable Interrupt bit */
+	rcar_dab_update_bits_locked(dab, RCAR_DAB_INTCR1,
+				    RCAR_DAB_INTCR1_FFT_DONE_MASK,
+				    RCAR_DAB_INTCR1_FFT_DONE_INT_ENABLED);
+
+	/* Enable FFT */
+	rcar_dab_write(dab, RCAR_DAB_FFTCR, RCAR_DAB_FFTCR_FFT_EN_ENABLED);
+
+	/* Mark FFT request as active */
+	rcar_dab_write(dab, RCAR_DAB_FFTREQCR,
+		       RCAR_DAB_FFTREQCR_FFT_REQ_ACTIVE);
+}
+
+void rcar_dab_fft_irq(struct rcar_dab *dab)
+{
+	rcar_dab_fft_disable(dab);
+	dab->fft.done = true;
+	wake_up_interruptible(&dab->fft.wait);
+}
+
+int rcar_dab_fft(struct rcar_dab *dab, struct rcar_dab_fft_req __user *fft_req)
+{
+	int buffer_size, ret;
+
+	buffer_size = fft_req->points * 4;
+
+	mutex_lock(&dab->fft.lock);
+
+	if (copy_from_user(dab->fft.input_buffer, fft_req->input_address,
+			   buffer_size)) {
+		mutex_unlock(&dab->fft.lock);
+		return -EFAULT;
+	}
+
+	dab->fft.done = false;
+	ret = rcar_dab_fft_init(dab, fft_req);
+	if (ret) {
+		mutex_unlock(&dab->fft.lock);
+		return ret;
+	}
+
+	rcar_dab_fft_enable(dab);
+	wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done, HZ);
+	if (!dab->fft.done) {
+		rcar_dab_fft_disable(dab);
+		ret = -EFAULT;
+	} else if (copy_to_user(fft_req->output_address, dab->fft.output_buffer,
+				buffer_size)) {
+		ret = -EFAULT;
+	}
+
+	mutex_unlock(&dab->fft.lock);
+
+	return ret;
+}
diff --git a/include/uapi/linux/rcar_dab.h b/include/uapi/linux/rcar_dab.h
new file mode 100644
index 000000000000..7f1ef07e3b57
--- /dev/null
+++ b/include/uapi/linux/rcar_dab.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * R-Car Gen3 DAB user space interface data structures
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#ifndef _RCAR_DAB_H_
+#define _RCAR_DAB_H_
+
+struct rcar_dab_fft_req {
+	int points;			/*
+					 * The number of points to use.
+					 * Legal values are 256, 512, 1024, and
+					 * 2048.
+					 */
+	unsigned char ofdm_number;	/*
+					 * Orthogonal Frequency Division
+					 * Multiplexing (OFDM).
+					 * Minimum value is 1, maximum value is
+					 * 255.
+					 */
+	void __user *input_address;	/*
+					 * User space address for the input
+					 * buffer.
+					 */
+	void __user *output_address;	/*
+					 * User space address for the output
+					 * buffer.
+					 */
+};
+
+#define	RCAR_DAB_IOC_FFT		_IOWR(0x90, 1, struct rcar_dab_fft_req *)
+
+#endif /* _RCAR_DAB_H_ */
-- 
2.25.1

[PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:56:31

R-Car M3-N (a.k.a. r8a77965) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 657b20d3533b..b6176fd5b703 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1013,6 +1013,18 @@ ipmmu_vp0: iommu@fe990000 {
 			#iommu-cells = <1>;
 		};
 
+		dab: dab@e6730000 {
+			compatible = "renesas,dab-r8a77965",
+				     "renesas,rcar-gen3-dab";
+			reg = <0x00 0xe6730000 0x00 0x120>;
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1016>;
+			clock-names = "dab";
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 1016>;
+			status = "disabled";
+		};
+
 		avb: ethernet@e6800000 {
 			compatible = "renesas,etheravb-r8a77965",
 				     "renesas,etheravb-rcar-gen3";
-- 
2.25.1

[PATCH 7/7] arm64: configs: Add R-Car DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-02-25 22:56:50

Make sure that the R-Car DAB device driver gets compiled as a
module since R-Car E3 and R-Car M3-N come with the DAB IP.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index d612f633b771..3b9996c7f1fc 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -274,6 +274,7 @@ CONFIG_PCI_ENDPOINT_TEST=m
 CONFIG_EEPROM_AT24=m
 CONFIG_EEPROM_AT25=m
 CONFIG_UACCE=m
+CONFIG_RCAR_DAB=m
 # CONFIG_SCSI_PROC_FS is not set
 CONFIG_BLK_DEV_SD=y
 CONFIG_SCSI_SAS_ATA=y
-- 
2.25.1

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: kernel test robot <hidden>
Date: 2021-02-26 04:58:57

Hi Fabrizio,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on robh/for-next renesas-drivers/renesas-clk renesas-devel/next v5.11 next-20210226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Fabrizio-Castro/Add-FFT-Support-for-R-Car-Gen3-devices/20210226-065737
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 29c395c77a9a514c5857c45ceae2665e9bd99ac7
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7f7ea7f057fde0d61b09f379dbd6abec9d00f713
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Fabrizio-Castro/Add-FFT-Support-for-R-Car-Gen3-devices/20210226-065737
        git checkout 7f7ea7f057fde0d61b09f379dbd6abec9d00f713
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):
quoted
drivers/misc/rcar_dab/rcar_dev.c:20:13: warning: no previous prototype for 'rcar_dab_irq' [-Wmissing-prototypes]
      20 | irqreturn_t rcar_dab_irq(int irq, void *devid)
         |             ^~~~~~~~~~~~


vim +/rcar_dab_irq +20 drivers/misc/rcar_dab/rcar_dev.c

    19	
  > 20	irqreturn_t rcar_dab_irq(int irq, void *devid)
    21	{
    22		struct rcar_dab *dab = devid;
    23		u32 intsr, intcr1;
    24	
    25		spin_lock(&dab->shared_regs_lock);
    26	
    27		intcr1 = rcar_dab_read(dab, RCAR_DAB_INTCR1);
    28		rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x000003FF);
    29	
    30		intsr = rcar_dab_read(dab, RCAR_DAB_INTSR);
    31		if (!intsr) {
    32			rcar_dab_write(dab, RCAR_DAB_INTCR1, intcr1);
    33			spin_unlock(&dab->shared_regs_lock);
    34			return IRQ_NONE;
    35		}
    36	
    37		/* Re-enable interrupts that haven't fired */
    38		rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x3FF & (intsr | intcr1));
    39		/* Clear interrupts */
    40		rcar_dab_write(dab, RCAR_DAB_INTSR, 0x7 & ~intsr);
    41	
    42		spin_unlock(&dab->shared_regs_lock);
    43	
    44		if (intsr & RCAR_DAB_INTSR_FFT_DONE)
    45			rcar_dab_fft_irq(dab);
    46	
    47		return IRQ_HANDLED;
    48	}
    49	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 08:36:24

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:52 PM Fabrizio Castro
[off-list ref] wrote:
This patch adds the DAB clock to the R8A77990 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted hunk
--- a/drivers/clk/renesas/r8a77990-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77990-cpg-mssr.c
@@ -232,6 +232,7 @@ static const struct mssr_mod_clk r8a77990_mod_clks[] __initconst = {
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77990_CLK_S3D1),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.
        DEF_MOD("scu-all",              1017,   R8A77990_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
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

Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 08:42:30

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted hunk
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
Please use the recommended order for new bindings:

    renesas,r8a77965-dab
    renesas,r8a77990-dab
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3 devices
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: dab
fck?

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

Re: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 08:46:37

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
This patch adds the DAB clock to the R8A77965 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted hunk
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -250,6 +250,7 @@ static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77965_CLK_S0D6),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.

While it's not unusual that the same module on R-Car E3 and M3-N
has different parent clocks, it does strike me as odd that S0D6 on M3-N
runs at 133 MHz, while S3D1 on E3 runs at 266 MHz.
Probably it doesn't matter that much, as your driver doesn't care
about the actual clock rate.
        DEF_MOD("scu-all",              1017,   R8A77965_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
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

Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Sergei Shtylyov <hidden>
Date: 2021-02-26 09:09:08

Hello!

On 26.02.2021 1:51, Fabrizio Castro wrote:
quoted hunk
Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
  .../devicetree/bindings/misc/renesas,dab.yaml | 75 +++++++++++++++++++
  1 file changed, 75 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/misc/renesas,dab.yaml
diff --git a/Documentation/devicetree/bindings/misc/renesas,dab.yaml b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
new file mode 100644
index 000000000000..e9494add13d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Renesas Electronics Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/renesas,dab.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car DAB Hardware Accelerator
+
+maintainers:
+  - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+
+description:
+  The DAB hardware accelerator found on some R-Car devices is a hardware
+  accelerator for software DAB demodulators.
+  It consists of one FFT (Fast Fourier Transform) module and one decoder module,
+  compatible with DAB specification (ETSI EN 300 401 and ETSI TS 102 563).
+  The decoder module can perform FIC decoding and MSC decoding processing from
+  de-puncture to final decoded result.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
    Why not renesas,<soc>-dab?
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3 devices
[...]

MBR, Sergei

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 09:35:49

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted hunk
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -461,4 +461,5 @@ source "drivers/misc/bcm-vk/Kconfig"
 source "drivers/misc/cardreader/Kconfig"
 source "drivers/misc/habanalabs/Kconfig"
 source "drivers/misc/uacce/Kconfig"
+source "drivers/misc/rcar_dab/Kconfig"
rcar-dab?

quoted hunk
--- /dev/null
+++ b/drivers/misc/rcar_dab/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# R-Car DAB Hardware Accelerator
+#
+
+config RCAR_DAB
DAB_RCAR?
+       tristate "DAB accelerator for Renesas R-Car devices"
+       depends on ARCH_R8A77990 || ARCH_R8A77965
|| COMPILE_TEST
+       help
+         Some R-Car devices come with an IP to accelerate DAB decoding.
+         Enable this option to add driver support.
quoted hunk
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_dev.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * R-Car Gen3 DAB hardware accelerator driver
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+
+#include <uapi/linux/rcar_dab.h>
+#include "rcar_dev.h"
+
+irqreturn_t rcar_dab_irq(int irq, void *devid)
static, as discovered by the robot.
+{
+       struct rcar_dab *dab = devid;
+       u32 intsr, intcr1;
+
+       spin_lock(&dab->shared_regs_lock);
+
+       intcr1 = rcar_dab_read(dab, RCAR_DAB_INTCR1);
+       rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x000003FF);
Magic value (setting undocumented bits?), please use a define.
+
+       intsr = rcar_dab_read(dab, RCAR_DAB_INTSR);
+       if (!intsr) {
+               rcar_dab_write(dab, RCAR_DAB_INTCR1, intcr1);
+               spin_unlock(&dab->shared_regs_lock);
+               return IRQ_NONE;
+       }
+
+       /* Re-enable interrupts that haven't fired */
+       rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x3FF & (intsr | intcr1));
+       /* Clear interrupts */
+       rcar_dab_write(dab, RCAR_DAB_INTSR, 0x7 & ~intsr);
More magic values.
+
+       spin_unlock(&dab->shared_regs_lock);
+
+       if (intsr & RCAR_DAB_INTSR_FFT_DONE)
+               rcar_dab_fft_irq(dab);
+
+       return IRQ_HANDLED;
+}
+
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int cmd,
+                                   unsigned long arg)
+{
+       void __user *argp = (void __user *)arg;
+       struct rcar_dab *dab;
+       int ret;
+
+       dab = container_of(file->private_data, struct rcar_dab, misc);
+
+       switch (cmd) {
+       case RCAR_DAB_IOC_FFT:
+               if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+                       return -EFAULT;
+               ret = rcar_dab_fft(dab, argp);
Do you envision ever using the FFT operation from kernel space?
Might be easier if you handle the copy_{from,to}_user() here.
+               break;
+       default:
+               ret = -ENOTTY;
+       }
+
+       return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+       .owner          = THIS_MODULE,
+       .unlocked_ioctl = rcar_dab_unlocked_ioctl,
Does this need compat_ioctl handling?
+};
+
+static int rcar_dab_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct rcar_dab *dab;
+       int ret, irq;
+
+       dab = devm_kzalloc(dev, sizeof(*dab), GFP_KERNEL);
+       if (!dab)
+               return -ENOMEM;
+       dab->pdev = pdev;
+
+       dab->base = devm_platform_ioremap_resource(pdev, 0);
+       if (IS_ERR(dab->base))
+               return PTR_ERR(dab->base);
+
+       irq = platform_get_irq(pdev, 0);
+       if (irq < 0) {
+               dev_err(dev, "Can't get the irq.\n");
+               return -EINVAL;
+       }
+
+       dab->clk = devm_clk_get(&pdev->dev, "dab");
+       if (IS_ERR(dab->clk)) {
+               ret = PTR_ERR(dab->clk);
+               dev_err(dev, "Can't get the clock (%d).\n", ret);
+               return ret;
+       }
+
+       spin_lock_init(&dab->shared_regs_lock);
+
+       ret = clk_prepare_enable(dab->clk);
Does the module clock need to be enabled all the time?
What about using Runtime PM instead of explicit clock handling, so your
driver will keep on working on future SoCs where the DAB block is part of
a power domain?
+       if (ret)
+               return ret;
+
+       ret = rcar_dab_fft_probe(dab);
+       if (ret)
+               goto error_clock_disable;
+
+       ret = devm_request_irq(dev, irq, rcar_dab_irq, 0, dev_name(dev), dab);
+       if (ret) {
+               dev_err(dev, "Can't request the irq (%d).\n", ret);
+               goto error_remove;
+       }
+
+       platform_set_drvdata(pdev, dab);
+
+       dab->misc.minor = MISC_DYNAMIC_MINOR;
+       dab->misc.name = RCAR_DAB_DRV_NAME;
+       dab->misc.fops = &rcar_dab_fops;
+       ret = misc_register(&dab->misc);
+       if (ret) {
+               dev_err(dev, "Can't register misc device (%d).\n", ret);
+               goto error_remove;
+       }
+
+       dev_info(dev, "R-Car Gen3 DAB misc driver ready.\n");
+
+       return 0;
+
+error_remove:
+       rcar_dab_fft_remove(dab);
+
+error_clock_disable:
+       clk_disable_unprepare(dab->clk);
+
+       return ret;
+}
quoted hunk
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_dev.h
+/* Register DAB_FFTCR */
+#define RCAR_DAB_FFTCR_FFT_EN_DISABLED         0
+#define RCAR_DAB_FFTCR_FFT_EN_ENABLED          1
Do you need both?

#define RCAR_DAB_FFTCR_FFT_EN        BIT(0)
+
+/* Register DAB_FFTREQCR */
+#define RCAR_DAB_FFTREQCR_FFT_REQ_INACTIVE     0
+#define RCAR_DAB_FFTREQCR_FFT_REQ_ACTIVE       1
Do you need both?
+
+/* Register DAB_INTSR */
+#define RCAR_DAB_INTSR_FFT_DONE                        0x1
BIT(0) (there are more bits for FIC and MSC)
+
+/* Register DAB_INTCR1 */
+#define RCAR_DAB_INTCR1_FFT_DONE_MASK          0x1
BIT(0) (there are more bits for FIC and MSC)
+#define RCAR_DAB_INTCR1_FFT_DONE_INT_ENABLED   0
+#define RCAR_DAB_INTCR1_FFT_DONE_INT_DISABLED  1
Do you need these?
I'd just retain RCAR_DAB_INTCR1_FFT_DONE.

For enabling interrupts:

    rcar_dab_update_bits_locked(dab, RCAR_DAB_INTCR1,
                                RCAR_DAB_INTCR1_FFT_DONE,
                                RCAR_DAB_INTCR1_FFT_DONE);

and for disabling:

    rcar_dab_update_bits_locked(dab, RCAR_DAB_INTCR1,
                                CAR_DAB_INTCR1_FFT_DONE, 0);
+
+struct rcar_dab_fft {
+       bool done;                      /*
+                                        * Condition for waking up the process
+                                        * sleeping while FFT is in progress.
+                                        */
Please use kerneldoc for documenting structures.
+       wait_queue_head_t wait;         /* Wait queue for FFT. */
+       struct mutex lock;              /* Mutex for FFT. */
+       dma_addr_t dma_input_buf;       /*
+                                        * Input buffer seen by the FFT
+                                        * module.
+                                        */
+       dma_addr_t dma_output_buf;      /*
+                                        * Output buffer seen by the FFT
+                                        * module.
+                                        */
+       void *input_buffer;             /* Input buffer seen by the CPU */
+       void *output_buffer;            /* Output buffer seen by the CPU */
Please use consistent naming (buf vs buffer).
+};
quoted hunk
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_fft.c
+int rcar_dab_fft(struct rcar_dab *dab, struct rcar_dab_fft_req __user *fft_req)
+{
+       int buffer_size, ret;
+
+       buffer_size = fft_req->points * 4;
Missing validation of buffer_size?
+
+       mutex_lock(&dab->fft.lock);
+
+       if (copy_from_user(dab->fft.input_buffer, fft_req->input_address,
+                          buffer_size)) {
+               mutex_unlock(&dab->fft.lock);
+               return -EFAULT;
+       }
+
+       dab->fft.done = false;
You can init done in rcar_dab_fft_init(), too.
+       ret = rcar_dab_fft_init(dab, fft_req);
+       if (ret) {
+               mutex_unlock(&dab->fft.lock);
+               return ret;
+       }
+
+       rcar_dab_fft_enable(dab);
+       wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done, HZ);
+       if (!dab->fft.done) {
You can just check the return value of wait_event_interruptible_timeout().
+               rcar_dab_fft_disable(dab);
+               ret = -EFAULT;
-ETIMEOUT?
+       } else if (copy_to_user(fft_req->output_address, dab->fft.output_buffer,
+                               buffer_size)) {
+               ret = -EFAULT;
+       }
+
+       mutex_unlock(&dab->fft.lock);
+
+       return ret;
+}
quoted hunk
--- /dev/null
+++ b/include/uapi/linux/rcar_dab.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * R-Car Gen3 DAB user space interface data structures
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#ifndef _RCAR_DAB_H_
+#define _RCAR_DAB_H_
+
+struct rcar_dab_fft_req {
+       int points;                     /*
unsigned int
+                                        * The number of points to use.
+                                        * Legal values are 256, 512, 1024, and
+                                        * 2048.
+                                        */
Please use kerneldoc to document struct members.
+       unsigned char ofdm_number;      /*
+                                        * Orthogonal Frequency Division
+                                        * Multiplexing (OFDM).
+                                        * Minimum value is 1, maximum value is
+                                        * 255.
+                                        */
Please make padding explicit.
I'd also sort the members by decreasing size, i.e. pointers first.
+       void __user *input_address;     /*
input_buf?
+                                        * User space address for the input
+                                        * buffer.
+                                        */
+       void __user *output_address;    /*
output_buf?
+                                        * User space address for the output
+                                        * buffer.
+                                        */
+};
+
+#define        RCAR_DAB_IOC_FFT                _IOWR(0x90, 1, struct rcar_dab_fft_req *)
+
+#endif /* _RCAR_DAB_H_ */
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

Re: [PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 09:39:08

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
R-Car M3-N (a.k.a. r8a77965) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
LGTM (ignoring clock-names bikeshedding)
quoted hunk
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1013,6 +1013,18 @@ ipmmu_vp0: iommu@fe990000 {
                        #iommu-cells = <1>;
                };

+               dab: dab@e6730000 {
+                       compatible = "renesas,dab-r8a77965",
+                                    "renesas,rcar-gen3-dab";
+                       reg = <0x00 0xe6730000 0x00 0x120>;
+                       interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&cpg CPG_MOD 1016>;
+                       clock-names = "dab";
+                       power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+                       resets = <&cpg 1016>;
+                       status = "disabled";
+               };
+
                avb: ethernet@e6800000 {
                        compatible = "renesas,etheravb-r8a77965",
                                     "renesas,etheravb-rcar-gen3";
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

Re: [PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 09:39:29

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
R-Car E3 (a.k.a. r8a77990) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
LGTM (ignoring clock-names bikeshedding)

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

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-02-26 10:40:03

On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro
[off-list ref] wrote:
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176 +++++++++++++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?

I don't think we want a custom ioctl interface for a device that implements
a generic specification. My first feeling would be that this should not
have a user-level API but instead get called by the DAB radio driver.

What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int cmd,
+                                   unsigned long arg)
+{
+       void __user *argp = (void __user *)arg;
+       struct rcar_dab *dab;
+       int ret;
+
+       dab = container_of(file->private_data, struct rcar_dab, misc);
+
+       switch (cmd) {
+       case RCAR_DAB_IOC_FFT:
+               if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+                       return -EFAULT;
+               ret = rcar_dab_fft(dab, argp);
+               break;
+       default:
+               ret = -ENOTTY;
+       }
+
+       return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+       .owner          = THIS_MODULE,
+       .unlocked_ioctl = rcar_dab_unlocked_ioctl,
+};
There should be a '.compat_ioctl = compat_ptr_ioctl'
entry, provided that the arguments are compatible between
32-bit and 64-bit user space.
+
+static int rcar_dab_fft_init(struct rcar_dab *dab, struct rcar_dab_fft_req *fft)
+{
+       u32 mode;
+
+       for (mode = 0; mode < ARRAY_SIZE(rcar_dab_fft_size_lut); mode++)
+               if (rcar_dab_fft_size_lut[mode] == fft->points)
+                       break;
+       if (mode == ARRAY_SIZE(rcar_dab_fft_size_lut))
+               return -EINVAL;
+       if (fft->ofdm_number == 0)
+               return -EINVAL;
+
+       rcar_dab_write(dab, RCAR_DAB_FFTSSR, mode);
+       rcar_dab_write(dab, RCAR_DAB_FFTNUMOFDMR, fft->ofdm_number);
+       rcar_dab_write(dab, RCAR_DAB_FFTINADDR, (u32)dab->fft.dma_input_buf);
+       rcar_dab_write(dab, RCAR_DAB_FFTOUTADDR, (u32)dab->fft.dma_output_buf);
Maybe use lower_32_bits() instead of the (u32) cast.

For clarity, you may also want to specifically ask for a 32-bit DMA mask
in the probe function, with a comment that describes what the hardware
limitation is.
+
+       if (copy_from_user(dab->fft.input_buffer, fft_req->input_address,
+                          buffer_size)) {
+               mutex_unlock(&dab->fft.lock);
+               return -EFAULT;
+       }
+
+       dab->fft.done = false;
+       ret = rcar_dab_fft_init(dab, fft_req);
+       if (ret) {
+               mutex_unlock(&dab->fft.lock);
+               return ret;
+       }
+
+       rcar_dab_fft_enable(dab);
+       wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done, HZ);
+       if (!dab->fft.done) {
+               rcar_dab_fft_disable(dab);
+               ret = -EFAULT;
-EFAULT doesn't look like the right error for timeout or signal
handling. Better check the return code from wait_event_interruptible_timeout()
instead.
+
+struct rcar_dab_fft_req {
+       int points;                     /*
+                                        * The number of points to use.
+                                        * Legal values are 256, 512, 1024, and
+                                        * 2048.
+                                        */
+       unsigned char ofdm_number;      /*
+                                        * Orthogonal Frequency Division
+                                        * Multiplexing (OFDM).
+                                        * Minimum value is 1, maximum value is
+                                        * 255.
+                                        */
+       void __user *input_address;     /*
+                                        * User space address for the input
+                                        * buffer.
+                                        */
+       void __user *output_address;    /*
+                                        * User space address for the output
+                                        * buffer.
+                                        */
+};
Please read Documentation/driver-api/ioctl.rst and make this a portable
data structure.

      Arnd

Re: [PATCH 0/7] Add FFT Support for R-Car Gen3 devices

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 12:22:16

Hi Fabrizio,

On Thu, Feb 25, 2021 at 10:51:40PM +0000, Fabrizio Castro wrote:
The DAB hardware accelerator found on R-Car E3 (a.k.a. r8a77990)
and R-Car M3-N (a.k.a. r8a77965) devices is a hardware accelerator
for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one
decoder module, compatible with DAB specification (ETSI EN 300 401
and ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing from de-puncture to final decoded result.

This series adds FFT support only for R-Car E3 and R-Car M3-N,
FIC and MSC support will be added later on.
Out of curiosity, could the FFT module be used as an accelerator for 2D
FFT on images ?
Fabrizio Castro (7):
  clk: renesas: r8a77990: Add DAB clock
  clk: renesas: r8a77965: Add DAB clock
  dt-bindings: misc: Add binding for R-Car DAB
  misc: Add driver for DAB IP found on Renesas R-Car devices
  arm64: dts: renesas: r8a77990: Add DAB support
  arm64: dts: renesas: r8a77965: Add DAB support
  arm64: configs: Add R-Car DAB support

 .../devicetree/bindings/misc/renesas,dab.yaml |  75 ++++++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/renesas/r8a77965.dtsi     |  12 ++
 arch/arm64/boot/dts/renesas/r8a77990.dtsi     |  12 ++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/clk/renesas/r8a77965-cpg-mssr.c       |   1 +
 drivers/clk/renesas/r8a77990-cpg-mssr.c       |   1 +
 drivers/misc/Kconfig                          |   1 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/rcar_dab/Kconfig                 |  11 ++
 drivers/misc/rcar_dab/Makefile                |   8 +
 drivers/misc/rcar_dab/rcar_dev.c              | 176 ++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h              | 116 ++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c              | 160 ++++++++++++++++
 include/uapi/linux/rcar_dab.h                 |  35 ++++
 15 files changed, 617 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/renesas,dab.yaml
 create mode 100644 drivers/misc/rcar_dab/Kconfig
 create mode 100644 drivers/misc/rcar_dab/Makefile
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.c
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.h
 create mode 100644 drivers/misc/rcar_dab/rcar_fft.c
 create mode 100644 include/uapi/linux/rcar_dab.h
-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 12:48:49

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:45PM +0000, Fabrizio Castro wrote:
quoted hunk
R-Car E3 (a.k.a. r8a77990) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 5010f23fafcc..5a6b835f137a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -963,6 +963,18 @@ ipmmu_vp0: iommu@fe990000 {
 			#iommu-cells = <1>;
 		};
 
+		dab: dab@e6730000 {
+			compatible = "renesas,dab-r8a77990",
+				     "renesas,rcar-gen3-dab";
+			reg = <0x00 0xe6730000 0x00 0x120>;
We often express the first cell as just 0.

			reg = <0 0xe6730000 0 0x120>;
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1016>;
As the clock isn't documented in the documentation, I can't verify this
of the resets property :-S
+			clock-names = "dab";
I'll comment on the clock name in the DT bindings.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 1016>;
+			status = "disabled";
+		};
+
 		avb: ethernet@e6800000 {
 			compatible = "renesas,etheravb-r8a77990",
 				     "renesas,etheravb-rcar-gen3";
-- 
Regards,

Laurent Pinchart

Re: [PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 12:49:25

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:46PM +0000, Fabrizio Castro wrote:
quoted hunk
R-Car M3-N (a.k.a. r8a77965) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 657b20d3533b..b6176fd5b703 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1013,6 +1013,18 @@ ipmmu_vp0: iommu@fe990000 {
 			#iommu-cells = <1>;
 		};
 
+		dab: dab@e6730000 {
+			compatible = "renesas,dab-r8a77965",
+				     "renesas,rcar-gen3-dab";
+			reg = <0x00 0xe6730000 0x00 0x120>;
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1016>;
+			clock-names = "dab";
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 1016>;
Same comments as for r8a77990.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+			status = "disabled";
+		};
+
 		avb: ethernet@e6800000 {
 			compatible = "renesas,etheravb-r8a77965",
 				     "renesas,etheravb-rcar-gen3";
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 12:50:25

Hi Fabrizio,

Thank you for the patch.

On Fri, Feb 26, 2021 at 09:45:20AM +0100, Geert Uytterhoeven wrote:
On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro wrote:
quoted
This patch adds the DAB clock to the R8A77965 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -250,6 +250,7 @@ static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77965_CLK_S0D6),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.

While it's not unusual that the same module on R-Car E3 and M3-N
has different parent clocks, it does strike me as odd that S0D6 on M3-N
runs at 133 MHz, while S3D1 on E3 runs at 266 MHz.
Probably it doesn't matter that much, as your driver doesn't care
about the actual clock rate.
I have the exact same concerns, here and for 1/7.
quoted
        DEF_MOD("scu-all",              1017,   R8A77965_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
-- 
Regards,

Laurent Pinchart

Re: [PATCH 7/7] arm64: configs: Add R-Car DAB support

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 12:54:30

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:47PM +0000, Fabrizio Castro wrote:
Make sure that the R-Car DAB device driver gets compiled as a
module since R-Car E3 and R-Car M3-N come with the DAB IP.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Do we need this in the defconfig ? It's not required to have a bootable
E3 or M3-N with the set of standard features, and would result in all
ARM64 platforms having one module they don't care about.
quoted hunk
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index d612f633b771..3b9996c7f1fc 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -274,6 +274,7 @@ CONFIG_PCI_ENDPOINT_TEST=m
 CONFIG_EEPROM_AT24=m
 CONFIG_EEPROM_AT25=m
 CONFIG_UACCE=m
+CONFIG_RCAR_DAB=m
 # CONFIG_SCSI_PROC_FS is not set
 CONFIG_BLK_DEV_SD=y
 CONFIG_SCSI_SAS_ATA=y
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 13:03:44

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:43PM +0000, Fabrizio Castro wrote:
quoted hunk
Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 .../devicetree/bindings/misc/renesas,dab.yaml | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/renesas,dab.yaml
diff --git a/Documentation/devicetree/bindings/misc/renesas,dab.yaml b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
new file mode 100644
index 000000000000..e9494add13d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Renesas Electronics Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/renesas,dab.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car DAB Hardware Accelerator
+
+maintainers:
+  - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+
+description:
+  The DAB hardware accelerator found on some R-Car devices is a hardware
+  accelerator for software DAB demodulators.
+  It consists of one FFT (Fast Fourier Transform) module and one decoder module,
+  compatible with DAB specification (ETSI EN 300 401 and ETSI TS 102 563).
+  The decoder module can perform FIC decoding and MSC decoding processing from
+  de-puncture to final decoded result.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3 devices
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
I usually try to describe clocks:

  clocks:
    items:
      - description: The module functional clock

but as there's a single clock, it may not be worth it. Up to you.
+
+  clock-names:
+    const: dab
With Geert's and Sergei's comments addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  # R8A77990 (R-Car E3)
+  - |
+    #include <dt-bindings/clock/r8a77990-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a77990-sysc.h>
+
+    dab: dab@e6730000 {
+        compatible = "renesas,dab-r8a77990",
+                     "renesas,rcar-gen3-dab";
+        reg = <0xe6730000 0x120>;
+        interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD 1016>;
+        clock-names = "dab";
+        power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+        resets = <&cpg 1016>;
+        status = "disabled";
+    };
-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-02-26 13:06:35

Hi Fabrizio,

Thank you for the patch.

On Fri, Feb 26, 2021 at 11:37:44AM +0100, Arnd Bergmann wrote:
On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176 +++++++++++++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?

I don't think we want a custom ioctl interface for a device that implements
a generic specification. My first feeling would be that this should not
have a user-level API but instead get called by the DAB radio driver.

What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
I second Arnd here, a standard API would be best.
quoted
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int cmd,
+                                   unsigned long arg)
+{
+       void __user *argp = (void __user *)arg;
+       struct rcar_dab *dab;
+       int ret;
+
+       dab = container_of(file->private_data, struct rcar_dab, misc);
+
+       switch (cmd) {
+       case RCAR_DAB_IOC_FFT:
+               if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+                       return -EFAULT;
+               ret = rcar_dab_fft(dab, argp);
+               break;
+       default:
+               ret = -ENOTTY;
+       }
+
+       return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+       .owner          = THIS_MODULE,
+       .unlocked_ioctl = rcar_dab_unlocked_ioctl,
+};
There should be a '.compat_ioctl = compat_ptr_ioctl'
entry, provided that the arguments are compatible between
32-bit and 64-bit user space.
quoted
+
+static int rcar_dab_fft_init(struct rcar_dab *dab, struct rcar_dab_fft_req *fft)
+{
+       u32 mode;
+
+       for (mode = 0; mode < ARRAY_SIZE(rcar_dab_fft_size_lut); mode++)
+               if (rcar_dab_fft_size_lut[mode] == fft->points)
+                       break;
+       if (mode == ARRAY_SIZE(rcar_dab_fft_size_lut))
+               return -EINVAL;
+       if (fft->ofdm_number == 0)
+               return -EINVAL;
+
+       rcar_dab_write(dab, RCAR_DAB_FFTSSR, mode);
+       rcar_dab_write(dab, RCAR_DAB_FFTNUMOFDMR, fft->ofdm_number);
+       rcar_dab_write(dab, RCAR_DAB_FFTINADDR, (u32)dab->fft.dma_input_buf);
+       rcar_dab_write(dab, RCAR_DAB_FFTOUTADDR, (u32)dab->fft.dma_output_buf);
Maybe use lower_32_bits() instead of the (u32) cast.

For clarity, you may also want to specifically ask for a 32-bit DMA mask
in the probe function, with a comment that describes what the hardware
limitation is.
quoted
+
+       if (copy_from_user(dab->fft.input_buffer, fft_req->input_address,
+                          buffer_size)) {
+               mutex_unlock(&dab->fft.lock);
+               return -EFAULT;
+       }
+
+       dab->fft.done = false;
+       ret = rcar_dab_fft_init(dab, fft_req);
+       if (ret) {
+               mutex_unlock(&dab->fft.lock);
+               return ret;
+       }
+
+       rcar_dab_fft_enable(dab);
+       wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done, HZ);
+       if (!dab->fft.done) {
+               rcar_dab_fft_disable(dab);
+               ret = -EFAULT;
-EFAULT doesn't look like the right error for timeout or signal
handling. Better check the return code from wait_event_interruptible_timeout()
instead.
quoted
+
+struct rcar_dab_fft_req {
+       int points;                     /*
+                                        * The number of points to use.
+                                        * Legal values are 256, 512, 1024, and
+                                        * 2048.
+                                        */
+       unsigned char ofdm_number;      /*
+                                        * Orthogonal Frequency Division
+                                        * Multiplexing (OFDM).
+                                        * Minimum value is 1, maximum value is
+                                        * 255.
+                                        */
+       void __user *input_address;     /*
+                                        * User space address for the input
+                                        * buffer.
+                                        */
+       void __user *output_address;    /*
+                                        * User space address for the output
+                                        * buffer.
+                                        */
+};
Please read Documentation/driver-api/ioctl.rst and make this a portable
data structure.
We've suffered enough with DMA to user pointers. Let's use dmabuf
instead.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 7/7] arm64: configs: Add R-Car DAB support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-02-26 13:07:06

Hi Laurent,

On Fri, Feb 26, 2021 at 1:53 PM Laurent Pinchart
[off-list ref] wrote:
On Thu, Feb 25, 2021 at 10:51:47PM +0000, Fabrizio Castro wrote:
quoted
Make sure that the R-Car DAB device driver gets compiled as a
module since R-Car E3 and R-Car M3-N come with the DAB IP.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Do we need this in the defconfig ? It's not required to have a bootable
E3 or M3-N with the set of standard features, and would result in all
That's why it's modular.
ARM64 platforms having one module they don't care about.
We typically add all drivers used by all supported platforms, to
increase build coverage.

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

RE: [PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:48:25

Hi Geert,

Thanks for your feedback!
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 08:35
Subject: Re: [PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:52 PM Fabrizio Castro
[off-list ref] wrote:
quoted
This patch adds the DAB clock to the R8A77990 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/clk/renesas/r8a77990-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77990-cpg-mssr.c
@@ -232,6 +232,7 @@ static const struct mssr_mod_clk r8a77990_mod_clks[]
__initconst = {
quoted
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77990_CLK_S3D1),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.
Yeah, unfortunately there is no official document with this information
as of yet.

Cheers,
Fab
quoted
        DEF_MOD("scu-all",              1017,   R8A77990_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
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

RE: [PATCH 0/7] Add FFT Support for R-Car Gen3 devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:51:47

Hi Laurent,

Thank you for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 12:21
Subject: Re: [PATCH 0/7] Add FFT Support for R-Car Gen3 devices

Hi Fabrizio,

On Thu, Feb 25, 2021 at 10:51:40PM +0000, Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 (a.k.a. r8a77990)
and R-Car M3-N (a.k.a. r8a77965) devices is a hardware accelerator
for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one
decoder module, compatible with DAB specification (ETSI EN 300 401
and ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing from de-puncture to final decoded result.

This series adds FFT support only for R-Car E3 and R-Car M3-N,
FIC and MSC support will be added later on.
Out of curiosity, could the FFT module be used as an accelerator for 2D
FFT on images ?
I haven't tried using it that way but I don't think so.

Thanks,
Fab 
quoted
Fabrizio Castro (7):
  clk: renesas: r8a77990: Add DAB clock
  clk: renesas: r8a77965: Add DAB clock
  dt-bindings: misc: Add binding for R-Car DAB
  misc: Add driver for DAB IP found on Renesas R-Car devices
  arm64: dts: renesas: r8a77990: Add DAB support
  arm64: dts: renesas: r8a77965: Add DAB support
  arm64: configs: Add R-Car DAB support

 .../devicetree/bindings/misc/renesas,dab.yaml |  75 ++++++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/renesas/r8a77965.dtsi     |  12 ++
 arch/arm64/boot/dts/renesas/r8a77990.dtsi     |  12 ++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/clk/renesas/r8a77965-cpg-mssr.c       |   1 +
 drivers/clk/renesas/r8a77990-cpg-mssr.c       |   1 +
 drivers/misc/Kconfig                          |   1 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/rcar_dab/Kconfig                 |  11 ++
 drivers/misc/rcar_dab/Makefile                |   8 +
 drivers/misc/rcar_dab/rcar_dev.c              | 176 ++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h              | 116 ++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c              | 160 ++++++++++++++++
 include/uapi/linux/rcar_dab.h                 |  35 ++++
 15 files changed, 617 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/misc/renesas,dab.yaml
quoted
 create mode 100644 drivers/misc/rcar_dab/Kconfig
 create mode 100644 drivers/misc/rcar_dab/Makefile
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.c
 create mode 100644 drivers/misc/rcar_dab/rcar_dev.h
 create mode 100644 drivers/misc/rcar_dab/rcar_fft.c
 create mode 100644 include/uapi/linux/rcar_dab.h
--
Regards,

Laurent Pinchart

RE: [PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:53:22

Hi Geert,

Thanks for your feedback!
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 09:38
Subject: Re: [PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
quoted
R-Car E3 (a.k.a. r8a77990) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
LGTM (ignoring clock-names bikeshedding)
Will change the clock name as per your comments.

Thanks,
Fab
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

RE: [PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:55:13

Hi Laurent,

Thanks for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 12:47
Subject: Re: [PATCH 5/7] arm64: dts: renesas: r8a77990: Add DAB support

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:45PM +0000, Fabrizio Castro wrote:
quoted
R-Car E3 (a.k.a. r8a77990) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
quoted
index 5010f23fafcc..5a6b835f137a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -963,6 +963,18 @@ ipmmu_vp0: iommu@fe990000 {
 			#iommu-cells = <1>;
 		};

+		dab: dab@e6730000 {
+			compatible = "renesas,dab-r8a77990",
+				     "renesas,rcar-gen3-dab";
+			reg = <0x00 0xe6730000 0x00 0x120>;
We often express the first cell as just 0.
I will replace 0x00 with 0.
			reg = <0 0xe6730000 0 0x120>;
quoted
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1016>;
As the clock isn't documented in the documentation, I can't verify this
of the resets property :-S
quoted
+			clock-names = "dab";
I'll comment on the clock name in the DT bindings.
Thanks,
Fab
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
quoted
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 1016>;
+			status = "disabled";
+		};
+
 		avb: ethernet@e6800000 {
 			compatible = "renesas,etheravb-r8a77990",
 				     "renesas,etheravb-rcar-gen3";
--
Regards,

Laurent Pinchart

RE: [PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:55:43

Hi Geert,

Thanks for you feedback!
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 09:37
Subject: Re: [PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
quoted
R-Car M3-N (a.k.a. r8a77965) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
LGTM (ignoring clock-names bikeshedding)
Will change the clock name as per your suggestion.

Thanks,
Fab
quoted
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1013,6 +1013,18 @@ ipmmu_vp0: iommu@fe990000 {
                        #iommu-cells = <1>;
                };

+               dab: dab@e6730000 {
+                       compatible = "renesas,dab-r8a77965",
+                                    "renesas,rcar-gen3-dab";
+                       reg = <0x00 0xe6730000 0x00 0x120>;
+                       interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&cpg CPG_MOD 1016>;
+                       clock-names = "dab";
+                       power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+                       resets = <&cpg 1016>;
+                       status = "disabled";
+               };
+
                avb: ethernet@e6800000 {
                        compatible = "renesas,etheravb-r8a77965",
                                     "renesas,etheravb-rcar-gen3";
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

RE: [PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:56:46

Hi Laurent,

Thanks for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 12:48
Subject: Re: [PATCH 6/7] arm64: dts: renesas: r8a77965: Add DAB support

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:46PM +0000, Fabrizio Castro wrote:
quoted
R-Car M3-N (a.k.a. r8a77965) comes with the DAB hardware accelerator.
This patch adds SoC specific support.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
quoted
index 657b20d3533b..b6176fd5b703 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1013,6 +1013,18 @@ ipmmu_vp0: iommu@fe990000 {
 			#iommu-cells = <1>;
 		};

+		dab: dab@e6730000 {
+			compatible = "renesas,dab-r8a77965",
+				     "renesas,rcar-gen3-dab";
+			reg = <0x00 0xe6730000 0x00 0x120>;
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1016>;
+			clock-names = "dab";
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 1016>;
Same comments as for r8a77990.
I will revise accordingly for v2.

Thanks,
Fab
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
quoted
+			status = "disabled";
+		};
+
 		avb: ethernet@e6800000 {
 			compatible = "renesas,etheravb-r8a77965",
 				     "renesas,etheravb-rcar-gen3";
--
Regards,

Laurent Pinchart

RE: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 14:59:52

Hi Geert,


Thanks for your feedback!
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 08:45
Subject: Re: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
quoted
This patch adds the DAB clock to the R8A77965 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -250,6 +250,7 @@ static const struct mssr_mod_clk r8a77965_mod_clks[]
__initconst = {
quoted
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77965_CLK_S0D6),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.

While it's not unusual that the same module on R-Car E3 and M3-N
has different parent clocks, it does strike me as odd that S0D6 on M3-N
runs at 133 MHz, while S3D1 on E3 runs at 266 MHz.
Probably it doesn't matter that much, as your driver doesn't care
about the actual clock rate.
I had the same concerns but we have received confirmation for this.
Hopefully an official document will be released soon.

Thanks,
Fab
quoted
        DEF_MOD("scu-all",              1017,   R8A77965_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
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

RE: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 15:02:21

Hi Laurent,

Thanks for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 12:49
Subject: Re: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

Hi Fabrizio,

Thank you for the patch.

On Fri, Feb 26, 2021 at 09:45:20AM +0100, Geert Uytterhoeven wrote:
quoted
On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro wrote:
quoted
This patch adds the DAB clock to the R8A77965 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -250,6 +250,7 @@ static const struct mssr_mod_clk
r8a77965_mod_clks[] __initconst = {
quoted
quoted
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77965_CLK_S0D6),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.

While it's not unusual that the same module on R-Car E3 and M3-N
has different parent clocks, it does strike me as odd that S0D6 on M3-N
runs at 133 MHz, while S3D1 on E3 runs at 266 MHz.
Probably it doesn't matter that much, as your driver doesn't care
about the actual clock rate.
I have the exact same concerns, here and for 1/7.
I hear you, but you know how these sort of things go, a bit was left
behind, now they are aware of this bit missing from the documentation
and at some point they will release a document addressing this particular
point. Meanwhile, we have received written confirmation for this, so
it should be okay.

Thanks,
Fab
quoted
quoted
        DEF_MOD("scu-all",              1017,   R8A77965_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
--
Regards,

Laurent Pinchart

RE: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 15:13:02

Hi Geert,

Thanks for your feedback!
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 08:41
Subject: Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
quoted
Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
quoted
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
Please use the recommended order for new bindings:

    renesas,r8a77965-dab
    renesas,r8a77990-dab
Sorry about this, will change.
quoted
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3
devices
quoted
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: dab
fck?
Agreed.

Thanks,
Fab
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

RE: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 15:15:04

Hello Sergei,

Thanks for your feedback!
From: Sergei Shtylyov <redacted>
Sent: 26 February 2021 09:07
Subject: Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

Hello!

On 26.02.2021 1:51, Fabrizio Castro wrote:
quoted
Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
  .../devicetree/bindings/misc/renesas,dab.yaml | 75 +++++++++++++++++++
  1 file changed, 75 insertions(+)
  create mode 100644
Documentation/devicetree/bindings/misc/renesas,dab.yaml
quoted
diff --git a/Documentation/devicetree/bindings/misc/renesas,dab.yaml
b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
quoted
new file mode 100644
index 000000000000..e9494add13d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Renesas Electronics Corporation
+%YAML 1.2
+---
+$id:
https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
e.org%2Fschemas%2Fmisc%2Frenesas%2Cdab.yaml%23&amp;data=04%7C01%7Cfabrizio
.castro.jz%40renesas.com%7C4ea11a8f46fc4f4642ad08d8da35d682%7C53d82571da19
47e49cb4625a166a4a2a%7C0%7C0%7C637499272091083125%7CUnknown%7CTWFpbGZsb3d8
eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&
amp;sdata=p5MNncCLOIMaYLyBtmOl%2Br%2BIKe9ByUqxv1k05FbYj94%3D&amp;reserved=
0
quoted
+$schema:
https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
e.org%2Fmeta-
schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cfabrizio.castro.jz%40renesas.com
%7C4ea11a8f46fc4f4642ad08d8da35d682%7C53d82571da1947e49cb4625a166a4a2a%7C0
%7C0%7C637499272091083125%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=dBI8WMPjxAEvhY
3cj7WKJW4FjCrfN5JpAoqC9XSPNaY%3D&amp;reserved=0
quoted
+
+title: Renesas R-Car DAB Hardware Accelerator
+
+maintainers:
+  - Fabrizio Castro [off-list ref]
+
+description:
+  The DAB hardware accelerator found on some R-Car devices is a
hardware
quoted
+  accelerator for software DAB demodulators.
+  It consists of one FFT (Fast Fourier Transform) module and one
decoder module,
quoted
+  compatible with DAB specification (ETSI EN 300 401 and ETSI TS 102
563).
quoted
+  The decoder module can perform FIC decoding and MSC decoding
processing from
quoted
+  de-puncture to final decoded result.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
    Why not renesas,<soc>-dab?
Will change.


Thanks,
Fab
quoted
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3
devices
[...]

MBR, Sergei

RE: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 15:16:27

Hi Laurent,

Thanks for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 13:02
Subject: Re: [PATCH 3/7] dt-bindings: misc: Add binding for R-Car DAB

Hi Fabrizio,

Thank you for the patch.

On Thu, Feb 25, 2021 at 10:51:43PM +0000, Fabrizio Castro wrote:
quoted
Document bindings for R-Car DAB hardware accelerator, currently
found on the r8a77990 SoC (a.k.a. R-Car E3) and on the r8a77965
SoC (a.k.a. R-Car M3-N).

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 .../devicetree/bindings/misc/renesas,dab.yaml | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/misc/renesas,dab.yaml
quoted
diff --git a/Documentation/devicetree/bindings/misc/renesas,dab.yaml
b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
quoted
new file mode 100644
index 000000000000..e9494add13d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/renesas,dab.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Renesas Electronics Corporation
+%YAML 1.2
+---
+$id:
https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
e.org%2Fschemas%2Fmisc%2Frenesas%2Cdab.yaml%23&amp;data=04%7C01%7Cfabrizio
.castro.jz%40renesas.com%7Cb383aa9cfef34b6653e008d8da56c204%7C53d82571da19
47e49cb4625a166a4a2a%7C0%7C0%7C637499413468300421%7CUnknown%7CTWFpbGZsb3d8
eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&
amp;sdata=r95fhwTCpf0gkNoRtCLrUbZtaCbI3da9sbFLv0UXipE%3D&amp;reserved=0
quoted
+$schema:
https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetre
e.org%2Fmeta-
schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cfabrizio.castro.jz%40renesas.com
%7Cb383aa9cfef34b6653e008d8da56c204%7C53d82571da1947e49cb4625a166a4a2a%7C0
%7C0%7C637499413468300421%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ajJ%2BmBs4zOzK
cJghYY89B6PMgrxRFgoVVuflZCFmHYc%3D&amp;reserved=0
quoted
+
+title: Renesas R-Car DAB Hardware Accelerator
+
+maintainers:
+  - Fabrizio Castro [off-list ref]
+
+description:
+  The DAB hardware accelerator found on some R-Car devices is a
hardware
quoted
+  accelerator for software DAB demodulators.
+  It consists of one FFT (Fast Fourier Transform) module and one
decoder module,
quoted
+  compatible with DAB specification (ETSI EN 300 401 and ETSI TS 102
563).
quoted
+  The decoder module can perform FIC decoding and MSC decoding
processing from
quoted
+  de-puncture to final decoded result.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,dab-r8a77965     # R-Car M3-N
+          - renesas,dab-r8a77990     # R-Car E3
+      - const: renesas,rcar-gen3-dab # Generic fallback for R-Car Gen3
devices
quoted
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
I usually try to describe clocks:

  clocks:
    items:
      - description: The module functional clock

but as there's a single clock, it may not be worth it. Up to you.
Will change as per your suggestion.

Thanks,
Fab
quoted
+
+  clock-names:
+    const: dab
With Geert's and Sergei's comments addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
quoted
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  # R8A77990 (R-Car E3)
+  - |
+    #include <dt-bindings/clock/r8a77990-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a77990-sysc.h>
+
+    dab: dab@e6730000 {
+        compatible = "renesas,dab-r8a77990",
+                     "renesas,rcar-gen3-dab";
+        reg = <0xe6730000 0x120>;
+        interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD 1016>;
+        clock-names = "dab";
+        power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+        resets = <&cpg 1016>;
+        status = "disabled";
+    };
--
Regards,

Laurent Pinchart

RE: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 16:44:42

Hi Geert,

Thanks for your feedback!
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 09:34
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro
[off-list ref] wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -461,4 +461,5 @@ source "drivers/misc/bcm-vk/Kconfig"
 source "drivers/misc/cardreader/Kconfig"
 source "drivers/misc/habanalabs/Kconfig"
 source "drivers/misc/uacce/Kconfig"
+source "drivers/misc/rcar_dab/Kconfig"
rcar-dab?
I will change the directory name
quoted
--- /dev/null
+++ b/drivers/misc/rcar_dab/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# R-Car DAB Hardware Accelerator
+#
+
+config RCAR_DAB
DAB_RCAR?
Agreed
quoted
+       tristate "DAB accelerator for Renesas R-Car devices"
+       depends on ARCH_R8A77990 || ARCH_R8A77965
|| COMPILE_TEST
Will do
quoted
+       help
+         Some R-Car devices come with an IP to accelerate DAB decoding.
+         Enable this option to add driver support.
quoted
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_dev.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * R-Car Gen3 DAB hardware accelerator driver
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+
+#include <uapi/linux/rcar_dab.h>
+#include "rcar_dev.h"
+
+irqreturn_t rcar_dab_irq(int irq, void *devid)
static, as discovered by the robot.
Yep
quoted
+{
+       struct rcar_dab *dab = devid;
+       u32 intsr, intcr1;
+
+       spin_lock(&dab->shared_regs_lock);
+
+       intcr1 = rcar_dab_read(dab, RCAR_DAB_INTCR1);
+       rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x000003FF);
Magic value (setting undocumented bits?), please use a define.
The documentation (Section 44B.2.21 of the R-Car Gen3 HW User
manual) says to write (reserved) bits 31 to 10 as 0, to write
(reserved) bits 9 to 3 as 1, and the remaining 3 bits (2 to 0),
are interrupt masking bits set to 1 in this case to temporarily
disable interrupts.

I can of course add a define for this.
quoted
+
+       intsr = rcar_dab_read(dab, RCAR_DAB_INTSR);
+       if (!intsr) {
+               rcar_dab_write(dab, RCAR_DAB_INTCR1, intcr1);
+               spin_unlock(&dab->shared_regs_lock);
+               return IRQ_NONE;
+       }
+
+       /* Re-enable interrupts that haven't fired */
+       rcar_dab_write(dab, RCAR_DAB_INTCR1, 0x3FF & (intsr | intcr1));
+       /* Clear interrupts */
+       rcar_dab_write(dab, RCAR_DAB_INTSR, 0x7 & ~intsr);
More magic values.
As per section 44B.2.20 from the R-Car Gen3 HW User Manual,
Bit 31 to 3 are reserved and reads and writes as 0, and a
0 has to be written to bits 2 to 0 to clear the interrupts.

I will use a define for this as well.
quoted
+
+       spin_unlock(&dab->shared_regs_lock);
+
+       if (intsr & RCAR_DAB_INTSR_FFT_DONE)
+               rcar_dab_fft_irq(dab);
+
+       return IRQ_HANDLED;
+}
+
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int
cmd,
quoted
+                                   unsigned long arg)
+{
+       void __user *argp = (void __user *)arg;
+       struct rcar_dab *dab;
+       int ret;
+
+       dab = container_of(file->private_data, struct rcar_dab, misc);
+
+       switch (cmd) {
+       case RCAR_DAB_IOC_FFT:
+               if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+                       return -EFAULT;
+               ret = rcar_dab_fft(dab, argp);
Do you envision ever using the FFT operation from kernel space?
Might be easier if you handle the copy_{from,to}_user() here.
Buffers are pre-allocated and shared among requests, therefore access to the
buffers has to be protected with a mutex. To keep things tidy (all FFT related
work in the FFT related source file) I prefer to keep this as it is, as FIC
and MSC will add more to the ioctl.
quoted
+               break;
+       default:
+               ret = -ENOTTY;
+       }
+
+       return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+       .owner          = THIS_MODULE,
+       .unlocked_ioctl = rcar_dab_unlocked_ioctl,
Does this need compat_ioctl handling?
Will add
quoted
+};
+
+static int rcar_dab_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct rcar_dab *dab;
+       int ret, irq;
+
+       dab = devm_kzalloc(dev, sizeof(*dab), GFP_KERNEL);
+       if (!dab)
+               return -ENOMEM;
+       dab->pdev = pdev;
+
+       dab->base = devm_platform_ioremap_resource(pdev, 0);
+       if (IS_ERR(dab->base))
+               return PTR_ERR(dab->base);
+
+       irq = platform_get_irq(pdev, 0);
+       if (irq < 0) {
+               dev_err(dev, "Can't get the irq.\n");
+               return -EINVAL;
+       }
+
+       dab->clk = devm_clk_get(&pdev->dev, "dab");
+       if (IS_ERR(dab->clk)) {
+               ret = PTR_ERR(dab->clk);
+               dev_err(dev, "Can't get the clock (%d).\n", ret);
+               return ret;
+       }
+
+       spin_lock_init(&dab->shared_regs_lock);
+
+       ret = clk_prepare_enable(dab->clk);
Does the module clock need to be enabled all the time?
No, it doesn't.
What about using Runtime PM instead of explicit clock handling, so your
driver will keep on working on future SoCs where the DAB block is part of
a power domain?
Can do, will switch to using Runtime PM.
quoted
+       if (ret)
+               return ret;
+
+       ret = rcar_dab_fft_probe(dab);
+       if (ret)
+               goto error_clock_disable;
+
+       ret = devm_request_irq(dev, irq, rcar_dab_irq, 0, dev_name(dev),
dab);
quoted
+       if (ret) {
+               dev_err(dev, "Can't request the irq (%d).\n", ret);
+               goto error_remove;
+       }
+
+       platform_set_drvdata(pdev, dab);
+
+       dab->misc.minor = MISC_DYNAMIC_MINOR;
+       dab->misc.name = RCAR_DAB_DRV_NAME;
+       dab->misc.fops = &rcar_dab_fops;
+       ret = misc_register(&dab->misc);
+       if (ret) {
+               dev_err(dev, "Can't register misc device (%d).\n", ret);
+               goto error_remove;
+       }
+
+       dev_info(dev, "R-Car Gen3 DAB misc driver ready.\n");
+
+       return 0;
+
+error_remove:
+       rcar_dab_fft_remove(dab);
+
+error_clock_disable:
+       clk_disable_unprepare(dab->clk);
+
+       return ret;
+}
quoted
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_dev.h
quoted
+/* Register DAB_FFTCR */
+#define RCAR_DAB_FFTCR_FFT_EN_DISABLED         0
+#define RCAR_DAB_FFTCR_FFT_EN_ENABLED          1
Do you need both?
We don't, I have just thought it made things clearer.
#define RCAR_DAB_FFTCR_FFT_EN        BIT(0)
quoted
+
+/* Register DAB_FFTREQCR */
+#define RCAR_DAB_FFTREQCR_FFT_REQ_INACTIVE     0
+#define RCAR_DAB_FFTREQCR_FFT_REQ_ACTIVE       1
Do you need both?
We don't, I have just thought it made things clearer.
quoted
+
+/* Register DAB_INTSR */
+#define RCAR_DAB_INTSR_FFT_DONE                        0x1
BIT(0) (there are more bits for FIC and MSC)
Will change
quoted
+
+/* Register DAB_INTCR1 */
+#define RCAR_DAB_INTCR1_FFT_DONE_MASK          0x1
BIT(0) (there are more bits for FIC and MSC)
Will change
quoted
+#define RCAR_DAB_INTCR1_FFT_DONE_INT_ENABLED   0
+#define RCAR_DAB_INTCR1_FFT_DONE_INT_DISABLED  1
Do you need these?
I'd just retain RCAR_DAB_INTCR1_FFT_DONE.
Agreed
For enabling interrupts:

    rcar_dab_update_bits_locked(dab, RCAR_DAB_INTCR1,
                                RCAR_DAB_INTCR1_FFT_DONE,
                                RCAR_DAB_INTCR1_FFT_DONE);

and for disabling:

    rcar_dab_update_bits_locked(dab, RCAR_DAB_INTCR1,
                                CAR_DAB_INTCR1_FFT_DONE, 0);
Okay
quoted
+
+struct rcar_dab_fft {
+       bool done;                      /*
+                                        * Condition for waking up the
process
quoted
+                                        * sleeping while FFT is in
progress.
quoted
+                                        */
Please use kerneldoc for documenting structures.
Ok
quoted
+       wait_queue_head_t wait;         /* Wait queue for FFT. */
+       struct mutex lock;              /* Mutex for FFT. */
+       dma_addr_t dma_input_buf;       /*
+                                        * Input buffer seen by the FFT
+                                        * module.
+                                        */
+       dma_addr_t dma_output_buf;      /*
+                                        * Output buffer seen by the FFT
+                                        * module.
+                                        */
+       void *input_buffer;             /* Input buffer seen by the CPU
*/
quoted
+       void *output_buffer;            /* Output buffer seen by the CPU
*/

Please use consistent naming (buf vs buffer).
Ok
quoted
+};
quoted
--- /dev/null
+++ b/drivers/misc/rcar_dab/rcar_fft.c
quoted
+int rcar_dab_fft(struct rcar_dab *dab, struct rcar_dab_fft_req __user
*fft_req)
quoted
+{
+       int buffer_size, ret;
+
+       buffer_size = fft_req->points * 4;
Missing validation of buffer_size?
Will add
quoted
+
+       mutex_lock(&dab->fft.lock);
+
+       if (copy_from_user(dab->fft.input_buffer, fft_req-
input_address,
+                          buffer_size)) {
+               mutex_unlock(&dab->fft.lock);
+               return -EFAULT;
+       }
+
+       dab->fft.done = false;
You can init done in rcar_dab_fft_init(), too.
Will move
quoted
+       ret = rcar_dab_fft_init(dab, fft_req);
+       if (ret) {
+               mutex_unlock(&dab->fft.lock);
+               return ret;
+       }
+
+       rcar_dab_fft_enable(dab);
+       wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done,
HZ);
quoted
+       if (!dab->fft.done) {
You can just check the return value of wait_event_interruptible_timeout().
Will change
quoted
+               rcar_dab_fft_disable(dab);
+               ret = -EFAULT;
-ETIMEOUT?
Yeah, better, thanks
quoted
+       } else if (copy_to_user(fft_req->output_address, dab-
fft.output_buffer,
+                               buffer_size)) {
+               ret = -EFAULT;
+       }
+
+       mutex_unlock(&dab->fft.lock);
+
+       return ret;
+}
quoted
--- /dev/null
+++ b/include/uapi/linux/rcar_dab.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * R-Car Gen3 DAB user space interface data structures
+ *
+ * Copyright (C) 2021 Renesas Electronics Corporation
+ *
+ */
+#ifndef _RCAR_DAB_H_
+#define _RCAR_DAB_H_
+
+struct rcar_dab_fft_req {
+       int points;                     /*
unsigned int
Will change
quoted
+                                        * The number of points to use.
+                                        * Legal values are 256, 512,
1024, and
quoted
+                                        * 2048.
+                                        */
Please use kerneldoc to document struct members.
Ok
quoted
+       unsigned char ofdm_number;      /*
+                                        * Orthogonal Frequency Division
+                                        * Multiplexing (OFDM).
+                                        * Minimum value is 1, maximum
value is
quoted
+                                        * 255.
+                                        */
Please make padding explicit.
Okay
I'd also sort the members by decreasing size, i.e. pointers first.
Okay
quoted
+       void __user *input_address;     /*
input_buf?
Sure
quoted
+                                        * User space address for the
input
quoted
+                                        * buffer.
+                                        */
+       void __user *output_address;    /*
output_buf?
Sure
quoted
+                                        * User space address for the
output
quoted
+                                        * buffer.
+                                        */
+};
+
+#define        RCAR_DAB_IOC_FFT                _IOWR(0x90, 1, struct
rcar_dab_fft_req *)
quoted
+
+#endif /* _RCAR_DAB_H_ */
Thanks,
Fab
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

RE: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 22:11:01

Hi Arnd,

Thanks for your feedback!
From: Arnd Bergmann <arnd@kernel.org>
Sent: 26 February 2021 10:38
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro
[off-list ref] wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176 +++++++++++++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?
I wasn't entirely sure were to put it to be honest as I couldn't find
anything similar, that's why "misc" for v1, to mainly get a feedback
and advice.
I don't think we want a custom ioctl interface for a device that
implements
a generic specification. My first feeling would be that this should not
have a user-level API but instead get called by the DAB radio driver.
I hear you, the trouble is that the modules of this IP should be seen
as part of a SW and HW processing pipeline.
Some of the SW components of the pipeline can be proprietary, and 
unfortunately we don't have access (yet) to a framework that allows us to
test and demonstrate the whole pipeline, for the moment we can only test
things in isolation. It'll take us a while to come up with a full solution
(or to have access to one), and in the meantime our SoCs come with an IP
that can't be used because there is no driver for it (yet).

After discussing things further with Geert and Laurent, I think they
are right in saying that the best path for this is probably to add this
driver under "drivers/staging" as an interim solution, so that the IP will
be accessible by developers, and when we have everything we need (a fully
fledged processing framework), we will able to integrate it better with
the other components (if possible).
What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
This IP is similar to a DMA to some extent, in the sense that it takes
input data from a buffer in memory and it writes output data onto a buffer
in memory, and of course it does some processing in between.
It's not directly connected to any other Radio related IP.
The user space application can read DAB IQ samples from the R-Car DRIF
interface (via driver drivers/media/platform/rcar_drif.c, or from other
sources since this IP is decoupled from DRIF), and then when it's time
to accelerate FFT, FIC, or MSC, it can request services to the DAB IP, so
that the app can go on and use the processor to do some work, while the DAB
IP processes things.
A framework to connect and exchange processing blocks (either SW or HW) and
interfaces is therefore vital to properly place a kernel implementation
in the great scheme of things, in its absence a simple driver can help
us and users to integrate DAB acceleration within an interim DAB processing
pipeline, and that will lead to better interfaces in the future.
quoted
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int
cmd,
quoted
+                                   unsigned long arg)
+{
+       void __user *argp = (void __user *)arg;
+       struct rcar_dab *dab;
+       int ret;
+
+       dab = container_of(file->private_data, struct rcar_dab, misc);
+
+       switch (cmd) {
+       case RCAR_DAB_IOC_FFT:
+               if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+                       return -EFAULT;
+               ret = rcar_dab_fft(dab, argp);
+               break;
+       default:
+               ret = -ENOTTY;
+       }
+
+       return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+       .owner          = THIS_MODULE,
+       .unlocked_ioctl = rcar_dab_unlocked_ioctl,
+};
There should be a '.compat_ioctl = compat_ptr_ioctl'
entry, provided that the arguments are compatible between
32-bit and 64-bit user space.
Will add
quoted
+
+static int rcar_dab_fft_init(struct rcar_dab *dab, struct
rcar_dab_fft_req *fft)
quoted
+{
+       u32 mode;
+
+       for (mode = 0; mode < ARRAY_SIZE(rcar_dab_fft_size_lut); mode++)
+               if (rcar_dab_fft_size_lut[mode] == fft->points)
+                       break;
+       if (mode == ARRAY_SIZE(rcar_dab_fft_size_lut))
+               return -EINVAL;
+       if (fft->ofdm_number == 0)
+               return -EINVAL;
+
+       rcar_dab_write(dab, RCAR_DAB_FFTSSR, mode);
+       rcar_dab_write(dab, RCAR_DAB_FFTNUMOFDMR, fft->ofdm_number);
+       rcar_dab_write(dab, RCAR_DAB_FFTINADDR, (u32)dab-
fft.dma_input_buf);
+       rcar_dab_write(dab, RCAR_DAB_FFTOUTADDR, (u32)dab-
fft.dma_output_buf);
Maybe use lower_32_bits() instead of the (u32) cast.

For clarity, you may also want to specifically ask for a 32-bit DMA mask
in the probe function, with a comment that describes what the hardware
limitation is.
Thanks.
quoted
+
+       if (copy_from_user(dab->fft.input_buffer, fft_req-
input_address,
+                          buffer_size)) {
+               mutex_unlock(&dab->fft.lock);
+               return -EFAULT;
+       }
+
+       dab->fft.done = false;
+       ret = rcar_dab_fft_init(dab, fft_req);
+       if (ret) {
+               mutex_unlock(&dab->fft.lock);
+               return ret;
+       }
+
+       rcar_dab_fft_enable(dab);
+       wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done,
HZ);
quoted
+       if (!dab->fft.done) {
+               rcar_dab_fft_disable(dab);
+               ret = -EFAULT;
-EFAULT doesn't look like the right error for timeout or signal
handling. Better check the return code from
wait_event_interruptible_timeout()
instead.
Will do
quoted
+
+struct rcar_dab_fft_req {
+       int points;                     /*
+                                        * The number of points to use.
+                                        * Legal values are 256, 512,
1024, and
quoted
+                                        * 2048.
+                                        */
+       unsigned char ofdm_number;      /*
+                                        * Orthogonal Frequency Division
+                                        * Multiplexing (OFDM).
+                                        * Minimum value is 1, maximum
value is
quoted
+                                        * 255.
+                                        */
+       void __user *input_address;     /*
+                                        * User space address for the
input
quoted
+                                        * buffer.
+                                        */
+       void __user *output_address;    /*
+                                        * User space address for the
output
quoted
+                                        * buffer.
+                                        */
+};
Please read Documentation/driver-api/ioctl.rst and make this a portable
data structure.
Thanks,
Fab

      Arnd

RE: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-01 23:13:59

Hi Laurent,

Thank you for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 13:05
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

Hi Fabrizio,

Thank you for the patch.

On Fri, Feb 26, 2021 at 11:37:44AM +0100, Arnd Bergmann wrote:
quoted
On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices
is
quoted
quoted
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing
quoted
quoted
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176
+++++++++++++++++++++++++++++++
quoted
quoted
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?

I don't think we want a custom ioctl interface for a device that
implements
quoted
a generic specification. My first feeling would be that this should not
have a user-level API but instead get called by the DAB radio driver.

What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
I second Arnd here, a standard API would be best.
quoted
quoted
+static long rcar_dab_unlocked_ioctl(struct file *file, unsigned int
cmd,
quoted
quoted
+                                   unsigned long arg)
+{
+       void __user *argp = (void __user *)arg;
+       struct rcar_dab *dab;
+       int ret;
+
+       dab = container_of(file->private_data, struct rcar_dab, misc);
+
+       switch (cmd) {
+       case RCAR_DAB_IOC_FFT:
+               if (!access_ok(argp, sizeof(struct rcar_dab_fft_req)))
+                       return -EFAULT;
+               ret = rcar_dab_fft(dab, argp);
+               break;
+       default:
+               ret = -ENOTTY;
+       }
+
+       return ret;
+}
+
+static const struct file_operations rcar_dab_fops = {
+       .owner          = THIS_MODULE,
+       .unlocked_ioctl = rcar_dab_unlocked_ioctl,
+};
There should be a '.compat_ioctl = compat_ptr_ioctl'
entry, provided that the arguments are compatible between
32-bit and 64-bit user space.
quoted
+
+static int rcar_dab_fft_init(struct rcar_dab *dab, struct
rcar_dab_fft_req *fft)
quoted
quoted
+{
+       u32 mode;
+
+       for (mode = 0; mode < ARRAY_SIZE(rcar_dab_fft_size_lut);
mode++)
quoted
quoted
+               if (rcar_dab_fft_size_lut[mode] == fft->points)
+                       break;
+       if (mode == ARRAY_SIZE(rcar_dab_fft_size_lut))
+               return -EINVAL;
+       if (fft->ofdm_number == 0)
+               return -EINVAL;
+
+       rcar_dab_write(dab, RCAR_DAB_FFTSSR, mode);
+       rcar_dab_write(dab, RCAR_DAB_FFTNUMOFDMR, fft->ofdm_number);
+       rcar_dab_write(dab, RCAR_DAB_FFTINADDR, (u32)dab-
fft.dma_input_buf);
quoted
+       rcar_dab_write(dab, RCAR_DAB_FFTOUTADDR, (u32)dab-
fft.dma_output_buf);

Maybe use lower_32_bits() instead of the (u32) cast.

For clarity, you may also want to specifically ask for a 32-bit DMA mask
in the probe function, with a comment that describes what the hardware
limitation is.
quoted
+
+       if (copy_from_user(dab->fft.input_buffer, fft_req-
input_address,
quoted
+                          buffer_size)) {
+               mutex_unlock(&dab->fft.lock);
+               return -EFAULT;
+       }
+
+       dab->fft.done = false;
+       ret = rcar_dab_fft_init(dab, fft_req);
+       if (ret) {
+               mutex_unlock(&dab->fft.lock);
+               return ret;
+       }
+
+       rcar_dab_fft_enable(dab);
+       wait_event_interruptible_timeout(dab->fft.wait, dab->fft.done,
HZ);
quoted
quoted
+       if (!dab->fft.done) {
+               rcar_dab_fft_disable(dab);
+               ret = -EFAULT;
-EFAULT doesn't look like the right error for timeout or signal
handling. Better check the return code from
wait_event_interruptible_timeout()
quoted
instead.
quoted
+
+struct rcar_dab_fft_req {
+       int points;                     /*
+                                        * The number of points to
use.
quoted
quoted
+                                        * Legal values are 256, 512,
1024, and
quoted
quoted
+                                        * 2048.
+                                        */
+       unsigned char ofdm_number;      /*
+                                        * Orthogonal Frequency
Division
quoted
quoted
+                                        * Multiplexing (OFDM).
+                                        * Minimum value is 1, maximum
value is
quoted
quoted
+                                        * 255.
+                                        */
+       void __user *input_address;     /*
+                                        * User space address for the
input
quoted
quoted
+                                        * buffer.
+                                        */
+       void __user *output_address;    /*
+                                        * User space address for the
output
quoted
quoted
+                                        * buffer.
+                                        */
+};
Please read Documentation/driver-api/ioctl.rst and make this a portable
data structure.
We've suffered enough with DMA to user pointers. Let's use dmabuf
instead.
Will give it a try

Thanks,
Fab

--
Regards,

Laurent Pinchart

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Date: 2021-03-02 11:59:08

Hello everyone,

On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro
[off-list ref] wrote:
Hi Arnd,

Thanks for your feedback!
quoted
From: Arnd Bergmann <arnd@kernel.org>
Sent: 26 February 2021 10:38
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro
[off-list ref] wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176 +++++++++++++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?
I wasn't entirely sure were to put it to be honest as I couldn't find
anything similar, that's why "misc" for v1, to mainly get a feedback
and advice.
quoted
I don't think we want a custom ioctl interface for a device that
implements
a generic specification. My first feeling would be that this should not
have a user-level API but instead get called by the DAB radio driver.
I hear you, the trouble is that the modules of this IP should be seen
as part of a SW and HW processing pipeline.
Some of the SW components of the pipeline can be proprietary, and
unfortunately we don't have access (yet) to a framework that allows us to
test and demonstrate the whole pipeline, for the moment we can only test
things in isolation. It'll take us a while to come up with a full solution
(or to have access to one), and in the meantime our SoCs come with an IP
that can't be used because there is no driver for it (yet).

After discussing things further with Geert and Laurent, I think they
are right in saying that the best path for this is probably to add this
driver under "drivers/staging" as an interim solution, so that the IP will
be accessible by developers, and when we have everything we need (a fully
fledged processing framework), we will able to integrate it better with
the other components (if possible).
quoted
What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
This IP is similar to a DMA to some extent, in the sense that it takes
input data from a buffer in memory and it writes output data onto a buffer
in memory, and of course it does some processing in between.
That sounds like something that can fit V4L2 MEM2MEM driver.
You queue two buffers and an operation, and then run a job.
It's not directly connected to any other Radio related IP.
The user space application can read DAB IQ samples from the R-Car DRIF
interface (via driver drivers/media/platform/rcar_drif.c, or from other
sources since this IP is decoupled from DRIF), and then when it's time
to accelerate FFT, FIC, or MSC, it can request services to the DAB IP, so
that the app can go on and use the processor to do some work, while the DAB
IP processes things.
A framework to connect and exchange processing blocks (either SW or HW) and
interfaces is therefore vital to properly place a kernel implementation
in the great scheme of things, in its absence a simple driver can help
I'm not entirely sure we are missing a framework. What's missing in
V4L2 MEM2MEM?

Before considering drivers/staging, it would be interesting to figure
out if V4L2 can do it as-is, and if not, discuss what's missing.

Thanks,
Ezequiel

RE: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-02 14:01:45

Hello Ezequiel,


Thanks for your feedback!
From: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Sent: 02 March 2021 11:17
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

Hello everyone,

On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro
[off-list ref] wrote:
quoted
Hi Arnd,

Thanks for your feedback!
quoted
From: Arnd Bergmann <arnd@kernel.org>
Sent: 26 February 2021 10:38
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas
R-
quoted
quoted
Car devices

On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro
[off-list ref] wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N
devices is
quoted
quoted
quoted
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one
decoder
quoted
quoted
quoted
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing
quoted
quoted
quoted
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176
+++++++++++++++++++++++++++++++
quoted
quoted
quoted
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?
I wasn't entirely sure were to put it to be honest as I couldn't find
anything similar, that's why "misc" for v1, to mainly get a feedback
and advice.
quoted
I don't think we want a custom ioctl interface for a device that
implements
a generic specification. My first feeling would be that this should
not
quoted
quoted
have a user-level API but instead get called by the DAB radio driver.
I hear you, the trouble is that the modules of this IP should be seen
as part of a SW and HW processing pipeline.
Some of the SW components of the pipeline can be proprietary, and
unfortunately we don't have access (yet) to a framework that allows us
to
quoted
test and demonstrate the whole pipeline, for the moment we can only test
things in isolation. It'll take us a while to come up with a full
solution
quoted
(or to have access to one), and in the meantime our SoCs come with an IP
that can't be used because there is no driver for it (yet).

After discussing things further with Geert and Laurent, I think they
are right in saying that the best path for this is probably to add this
driver under "drivers/staging" as an interim solution, so that the IP
will
quoted
be accessible by developers, and when we have everything we need (a
fully
quoted
fledged processing framework), we will able to integrate it better with
the other components (if possible).
quoted
What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
This IP is similar to a DMA to some extent, in the sense that it takes
input data from a buffer in memory and it writes output data onto a
buffer
quoted
in memory, and of course it does some processing in between.
That sounds like something that can fit V4L2 MEM2MEM driver.
You queue two buffers and an operation, and then run a job.
V4L2 MEM2MEM seems good for this in general, however the DAB IP is going
to be shared by multiple processing pipelines (as usually we have several
DRIF interfaces, and only 1 DAB IP), and for what concerns FFT specifically
there is only 1 FFT module inside the DAB IP.
My understanding is that the capabilities of V4L2 MEM2MEM devices are
configured for the specific pipeline, but in the this context user space
would have to continuously switch the capabilities of the DAB IP (at the
right moment) to allow processing for data streams requiring different
capabilities.

Am I wrong?
quoted
It's not directly connected to any other Radio related IP.
The user space application can read DAB IQ samples from the R-Car DRIF
interface (via driver drivers/media/platform/rcar_drif.c, or from other
sources since this IP is decoupled from DRIF), and then when it's time
to accelerate FFT, FIC, or MSC, it can request services to the DAB IP,
so
quoted
that the app can go on and use the processor to do some work, while the
DAB
quoted
IP processes things.
A framework to connect and exchange processing blocks (either SW or HW)
and
quoted
interfaces is therefore vital to properly place a kernel implementation
in the great scheme of things, in its absence a simple driver can help
I'm not entirely sure we are missing a framework. What's missing in
V4L2 MEM2MEM?
I was referring to a user space framework (I should have been more specific
with my previous email).
Before considering drivers/staging, it would be interesting to figure
out if V4L2 can do it as-is, and if not, discuss what's missing.
I think an interim solution would allow us and users to evaluate things a
little bit better, so that we can integrate this IP with V4L2 later on.

Thanks,
Fab
Thanks,
Ezequiel

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-03-02 14:10:40

Hi Fabrizio,

On Tue, Mar 02, 2021 at 12:20:17PM +0000, Fabrizio Castro wrote:
On 02 March 2021 11:17, Ezequiel Garcia wrote:
quoted
On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro wrote:
quoted
On 26 February 2021 10:38, Arnd Bergmann wrote:
quoted
On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N devices is
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one decoder
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding processing
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176 +++++++++++++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160 ++++++++++++++++++++++++++++
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?
I wasn't entirely sure were to put it to be honest as I couldn't find
anything similar, that's why "misc" for v1, to mainly get a feedback
and advice.
quoted
I don't think we want a custom ioctl interface for a device that
implements
a generic specification. My first feeling would be that this should not
have a user-level API but instead get called by the DAB radio driver.
I hear you, the trouble is that the modules of this IP should be seen
as part of a SW and HW processing pipeline.
Some of the SW components of the pipeline can be proprietary, and
unfortunately we don't have access (yet) to a framework that allows us to
test and demonstrate the whole pipeline, for the moment we can only test
things in isolation. It'll take us a while to come up with a full solution
(or to have access to one), and in the meantime our SoCs come with an IP
that can't be used because there is no driver for it (yet).

After discussing things further with Geert and Laurent, I think they
are right in saying that the best path for this is probably to add this
driver under "drivers/staging" as an interim solution, so that the IP will
be accessible by developers, and when we have everything we need (a fully
fledged processing framework), we will able to integrate it better with
the other components (if possible).
quoted
What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
This IP is similar to a DMA to some extent, in the sense that it takes
input data from a buffer in memory and it writes output data onto a buffer
in memory, and of course it does some processing in between.
That sounds like something that can fit V4L2 MEM2MEM driver.
You queue two buffers and an operation, and then run a job.
V4L2 MEM2MEM seems good for this in general, however the DAB IP is going
to be shared by multiple processing pipelines (as usually we have several
DRIF interfaces, and only 1 DAB IP), and for what concerns FFT specifically
there is only 1 FFT module inside the DAB IP.
My understanding is that the capabilities of V4L2 MEM2MEM devices are
configured for the specific pipeline, but in the this context user space
would have to continuously switch the capabilities of the DAB IP (at the
right moment) to allow processing for data streams requiring different
capabilities.

Am I wrong?
V4L2 M2M devices can be opened multiple times, but different processes,
which can configure the device in different ways, and submit jobs that
are then scheduled by the V4L2 M2M framework.
quoted
quoted
It's not directly connected to any other Radio related IP.
The user space application can read DAB IQ samples from the R-Car DRIF
interface (via driver drivers/media/platform/rcar_drif.c, or from other
sources since this IP is decoupled from DRIF), and then when it's time
to accelerate FFT, FIC, or MSC, it can request services to the DAB IP, so
that the app can go on and use the processor to do some work, while the DAB
IP processes things.
A framework to connect and exchange processing blocks (either SW or HW) and
interfaces is therefore vital to properly place a kernel implementation
in the great scheme of things, in its absence a simple driver can help
I'm not entirely sure we are missing a framework. What's missing in
V4L2 MEM2MEM?
I was referring to a user space framework (I should have been more specific
with my previous email).
quoted
Before considering drivers/staging, it would be interesting to figure
out if V4L2 can do it as-is, and if not, discuss what's missing.
I think an interim solution would allow us and users to evaluate things a
little bit better, so that we can integrate this IP with V4L2 later on.
-- 
Regards,

Laurent Pinchart

RE: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-03 13:50:47

Hi Laurent,

Thanks for your feedback!
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 02 March 2021 12:32
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

Hi Fabrizio,

On Tue, Mar 02, 2021 at 12:20:17PM +0000, Fabrizio Castro wrote:
quoted
On 02 March 2021 11:17, Ezequiel Garcia wrote:
quoted
On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro wrote:
quoted
On 26 February 2021 10:38, Arnd Bergmann wrote:
quoted
On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N
devices is
quoted
quoted
quoted
quoted
quoted
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one
decoder
quoted
quoted
quoted
quoted
quoted
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing
quoted
quoted
quoted
quoted
quoted
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 MAINTAINERS                      |   7 ++
 drivers/misc/Kconfig             |   1 +
 drivers/misc/Makefile            |   1 +
 drivers/misc/rcar_dab/Kconfig    |  11 ++
 drivers/misc/rcar_dab/Makefile   |   8 ++
 drivers/misc/rcar_dab/rcar_dev.c | 176
+++++++++++++++++++++++++++++++
quoted
quoted
quoted
quoted
quoted
 drivers/misc/rcar_dab/rcar_dev.h | 116 ++++++++++++++++++++
 drivers/misc/rcar_dab/rcar_fft.c | 160
++++++++++++++++++++++++++++
quoted
quoted
quoted
quoted
quoted
 include/uapi/linux/rcar_dab.h    |  35 ++++++
Can you explain why this is not in drivers/media/?
I wasn't entirely sure were to put it to be honest as I couldn't
find
quoted
quoted
quoted
anything similar, that's why "misc" for v1, to mainly get a feedback
and advice.
quoted
I don't think we want a custom ioctl interface for a device that
implements
a generic specification. My first feeling would be that this
should not
quoted
quoted
quoted
quoted
have a user-level API but instead get called by the DAB radio
driver.
quoted
quoted
quoted
I hear you, the trouble is that the modules of this IP should be
seen
quoted
quoted
quoted
as part of a SW and HW processing pipeline.
Some of the SW components of the pipeline can be proprietary, and
unfortunately we don't have access (yet) to a framework that allows
us to
quoted
quoted
quoted
test and demonstrate the whole pipeline, for the moment we can only
test
quoted
quoted
quoted
things in isolation. It'll take us a while to come up with a full
solution
quoted
quoted
quoted
(or to have access to one), and in the meantime our SoCs come with
an IP
quoted
quoted
quoted
that can't be used because there is no driver for it (yet).

After discussing things further with Geert and Laurent, I think they
are right in saying that the best path for this is probably to add
this
quoted
quoted
quoted
driver under "drivers/staging" as an interim solution, so that the
IP will
quoted
quoted
quoted
be accessible by developers, and when we have everything we need (a
fully
quoted
quoted
quoted
fledged processing framework), we will able to integrate it better
with
quoted
quoted
quoted
the other components (if possible).
quoted
What is the intended usage model here? I assume the idea is to
use it in an application that receives audio or metadata from DAB.
What driver do you use for that?
This IP is similar to a DMA to some extent, in the sense that it
takes
quoted
quoted
quoted
input data from a buffer in memory and it writes output data onto a
buffer
quoted
quoted
quoted
in memory, and of course it does some processing in between.
That sounds like something that can fit V4L2 MEM2MEM driver.
You queue two buffers and an operation, and then run a job.
V4L2 MEM2MEM seems good for this in general, however the DAB IP is going
to be shared by multiple processing pipelines (as usually we have
several
quoted
DRIF interfaces, and only 1 DAB IP), and for what concerns FFT
specifically
quoted
there is only 1 FFT module inside the DAB IP.
My understanding is that the capabilities of V4L2 MEM2MEM devices are
configured for the specific pipeline, but in the this context user space
would have to continuously switch the capabilities of the DAB IP (at the
right moment) to allow processing for data streams requiring different
capabilities.

Am I wrong?
V4L2 M2M devices can be opened multiple times, but different processes,
which can configure the device in different ways, and submit jobs that
are then scheduled by the V4L2 M2M framework.
I'll give it a try in due time then.

I think the clock related patches are worth merging.

Thanks,
Fab
quoted
quoted
quoted
It's not directly connected to any other Radio related IP.
The user space application can read DAB IQ samples from the R-Car
DRIF
quoted
quoted
quoted
interface (via driver drivers/media/platform/rcar_drif.c, or from
other
quoted
quoted
quoted
sources since this IP is decoupled from DRIF), and then when it's
time
quoted
quoted
quoted
to accelerate FFT, FIC, or MSC, it can request services to the DAB
IP, so
quoted
quoted
quoted
that the app can go on and use the processor to do some work, while
the DAB
quoted
quoted
quoted
IP processes things.
A framework to connect and exchange processing blocks (either SW or
HW) and
quoted
quoted
quoted
interfaces is therefore vital to properly place a kernel
implementation
quoted
quoted
quoted
in the great scheme of things, in its absence a simple driver can
help
quoted
quoted
I'm not entirely sure we are missing a framework. What's missing in
V4L2 MEM2MEM?
I was referring to a user space framework (I should have been more
specific
quoted
with my previous email).
quoted
Before considering drivers/staging, it would be interesting to figure
out if V4L2 can do it as-is, and if not, discuss what's missing.
I think an interim solution would allow us and users to evaluate things
a
quoted
little bit better, so that we can integrate this IP with V4L2 later on.
--
Regards,

Laurent Pinchart

RE: [PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-03 14:33:17

Hi Geert,
From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Sent: 01 March 2021 14:47
Subject: RE: [PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

Hi Geert,

Thanks for your feedback!
quoted
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 26 February 2021 08:35
Subject: Re: [PATCH 1/7] clk: renesas: r8a77990: Add DAB clock

Hi Fabrizio,

On Thu, Feb 25, 2021 at 11:52 PM Fabrizio Castro
[off-list ref] wrote:
quoted
This patch adds the DAB clock to the R8A77990 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/clk/renesas/r8a77990-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77990-cpg-mssr.c
@@ -232,6 +232,7 @@ static const struct mssr_mod_clk
r8a77990_mod_clks[]
quoted
__initconst = {
quoted
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77990_CLK_S3D1),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.
Yeah, unfortunately there is no official document with this information
as of yet.
I think this patch is worth taking.

Thanks,
Fab
Cheers,
Fab
quoted
quoted
        DEF_MOD("scu-all",              1017,   R8A77990_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
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
quoted
that.
                                -- Linus Torvalds

RE: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-03 14:51:24

Hi Geert,
From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Sent: 01 March 2021 15:01
Subject: RE: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

Hi Laurent,

Thanks for your feedback!
quoted
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 26 February 2021 12:49
Subject: Re: [PATCH 2/7] clk: renesas: r8a77965: Add DAB clock

Hi Fabrizio,

Thank you for the patch.

On Fri, Feb 26, 2021 at 09:45:20AM +0100, Geert Uytterhoeven wrote:
quoted
On Thu, Feb 25, 2021 at 11:53 PM Fabrizio Castro wrote:
quoted
This patch adds the DAB clock to the R8A77965 SoC.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Thanks for your patch!
quoted
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -250,6 +250,7 @@ static const struct mssr_mod_clk
r8a77965_mod_clks[] __initconst = {
quoted
quoted
        DEF_MOD("ssi2",                 1013,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi1",                 1014,   MOD_CLK_ID(1005)),
        DEF_MOD("ssi0",                 1015,   MOD_CLK_ID(1005)),
+       DEF_MOD("dab",                  1016,   R8A77965_CLK_S0D6),
Unfortunately this bit is not documented in the R-Car Gen3 Hardware
User's Manual, so I have to trust you on this.

While it's not unusual that the same module on R-Car E3 and M3-N
has different parent clocks, it does strike me as odd that S0D6 on M3-
N
quoted
quoted
runs at 133 MHz, while S3D1 on E3 runs at 266 MHz.
Probably it doesn't matter that much, as your driver doesn't care
about the actual clock rate.
I have the exact same concerns, here and for 1/7.
I hear you, but you know how these sort of things go, a bit was left
behind, now they are aware of this bit missing from the documentation
and at some point they will release a document addressing this particular
point. Meanwhile, we have received written confirmation for this, so
it should be okay.
I think this patch is worth taking.

Thanks,
Fab

Thanks,
Fab
quoted
quoted
quoted
        DEF_MOD("scu-all",              1017,   R8A77965_CLK_S3D4),
        DEF_MOD("scu-dvc1",             1018,   MOD_CLK_ID(1017)),
        DEF_MOD("scu-dvc0",             1019,   MOD_CLK_ID(1017)),
--
Regards,

Laurent Pinchart

Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-03-03 15:19:03

Hi Fabrizio,

On Wed, Mar 3, 2021 at 11:20 AM Fabrizio Castro
[off-list ref] wrote:
quoted
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 02 March 2021 12:32
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

On Tue, Mar 02, 2021 at 12:20:17PM +0000, Fabrizio Castro wrote:
quoted
On 02 March 2021 11:17, Ezequiel Garcia wrote:
quoted
On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro wrote:
quoted
On 26 February 2021 10:38, Arnd Bergmann wrote:
quoted
On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-N
devices is
quoted
quoted
quoted
quoted
quoted
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and one
decoder
quoted
quoted
quoted
quoted
quoted
module, compatible with DAB specification (ETSI EN 300 401 and
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing
quoted
quoted
quoted
quoted
quoted
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module only.

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
[...]
I think the clock related patches are worth merging.
Indeed. I had already marked them to be applied, after you received
confirmation about the parent clocks.

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

RE: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-Car devices

From: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Date: 2021-03-03 15:38:13

Hi Geert,
From: Geert Uytterhoeven <geert@linux-m68k.org>
Sent: 03 March 2021 10:27
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas R-
Car devices

Hi Fabrizio,

On Wed, Mar 3, 2021 at 11:20 AM Fabrizio Castro
[off-list ref] wrote:
quoted
quoted
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Sent: 02 March 2021 12:32
Subject: Re: [PATCH 4/7] misc: Add driver for DAB IP found on Renesas
R-
quoted
quoted
Car devices

On Tue, Mar 02, 2021 at 12:20:17PM +0000, Fabrizio Castro wrote:
quoted
On 02 March 2021 11:17, Ezequiel Garcia wrote:
quoted
On Mon, 1 Mar 2021 at 14:36, Fabrizio Castro wrote:
quoted
On 26 February 2021 10:38, Arnd Bergmann wrote:
quoted
On Thu, Feb 25, 2021 at 11:51 PM Fabrizio Castro wrote:
quoted
The DAB hardware accelerator found on R-Car E3 and R-Car M3-
N
quoted
quoted
devices is
quoted
quoted
quoted
quoted
quoted
a hardware accelerator for software DAB demodulators.
It consists of one FFT (Fast Fourier Transform) module and
one
quoted
quoted
decoder
quoted
quoted
quoted
quoted
quoted
module, compatible with DAB specification (ETSI EN 300 401
and
quoted
quoted
quoted
quoted
quoted
quoted
quoted
ETSI TS 102 563).
The decoder module can perform FIC decoding and MSC decoding
processing
quoted
quoted
quoted
quoted
quoted
from de-puncture to final decoded result.

This patch adds a device driver to support the FFT module
only.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
Signed-off-by: Fabrizio Castro
[off-list ref]

[...]
quoted
I think the clock related patches are worth merging.
Indeed. I had already marked them to be applied, after you received
confirmation about the parent clocks.
Thanks,
Fab
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help