[PATCH 0/2] media: rc: add support for Amlogic Meson IR blaster

[PATCH 0/2] media: rc: add support for Amlogic Meson IR blaster

[Viktor Prutyanov]

Hi,

this is a driver for the IR transmitter (also called IR blaster)
available in some Amlogic Meson SoCs.

Viktor Prutyanov (2):
  media: rc: meson-irblaster: document device tree bindings
  media: rc: introduce Meson IR blaster driver

 .../media/amlogic,meson-irblaster.yaml        |  63 +++
 drivers/media/rc/Kconfig                      |  10 +
 drivers/media/rc/Makefile                     |   1 +
 drivers/media/rc/meson-irblaster.c            | 433 ++++++++++++++++++
 4 files changed, 507 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/amlogic,meson-irblaster.yaml
 create mode 100644 drivers/media/rc/meson-irblaster.c

-- 
2.21.0

[PATCH 1/2] media: rc: meson-irblaster: document device tree bindings

[Viktor Prutyanov]

This patch adds binding documentation for the IR transmitter
available in Amlogic Meson SoCs.

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
---
 .../media/amlogic,meson-irblaster.yaml        | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/amlogic,meson-irblaster.yaml

diff --git a/Documentation/devicetree/bindings/media/amlogic,meson-irblaster.yaml b/Documentation/devicetree/bindings/media/amlogic,meson-irblaster.yaml
new file mode 100644
index 000000000000..baecda092a78
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/amlogic,meson-irblaster.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/media/amlogic,meson-irblaster.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson IR blaster
+
+maintainers:
+  - Viktor Prutyanov <viktor.prutyanov@phystech.edu>
+
+description: |
+  Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
+  (blaster) controller onboard. It is capable of sending IR signals
+  with arbitrary carrier frequency and duty cycle.
+
+properties:
+  compatible:
+    const: amlogic,meson-irblaster
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      - const: sysclk
+      - const: xtal
+
+  mod-clock:
+    oneOf:
+      - const: sysclk
+      - const: xtal
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/g12a-clkc.h>
+
+    meson-irblaster@ff80014c {
+      compatible = "amlogic,meson-irblaster";
+      reg = <0xff80014c 0x10>;
+      interrupts = <0 198 IRQ_TYPE_EDGE_RISING>;
+      clocks = <&clkc CLKID_CLK81 &xtal>;
+      clock-names = "sysclk", "xtal";
+      mod-clock = "xtal";
+    };
-- 
2.21.0

[PATCH 2/2] media: rc: introduce Meson IR blaster driver

[Viktor Prutyanov]

This patch adds the driver for Amlogic Meson IR blaster.

Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
(blaster) controller onboard. It is capable of sending IR
signals with arbitrary carrier frequency and duty cycle.

The driver supports 3 modulation clock sources:
 - sysclk
 - xtal3 clock (xtal divided by 3)
 - 1us clock

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
---
 drivers/media/rc/Kconfig           |  10 +
 drivers/media/rc/Makefile          |   1 +
 drivers/media/rc/meson-irblaster.c | 433 +++++++++++++++++++++++++++++
 3 files changed, 444 insertions(+)
 create mode 100644 drivers/media/rc/meson-irblaster.c

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index d0a8326b75c2..6e60348e1bcf 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -246,6 +246,16 @@ config IR_MESON
 	   To compile this driver as a module, choose M here: the
 	   module will be called meson-ir.
 
+config IR_MESON_IRBLASTER
+	tristate "Amlogic Meson IR blaster"
+	depends on ARCH_MESON || COMPILE_TEST
+	help
+	   Say Y if you want to use the IR blaster available on
+	   Amlogic Meson SoCs.
+
+	   To compile this driver as a module, choose M here: the
+	   module will be called meson-irblaster.
+
 config IR_MTK
 	tristate "Mediatek IR remote receiver"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 692e9b6b203f..b108f2b0420c 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_IR_ITE_CIR) += ite-cir.o
 obj-$(CONFIG_IR_MCEUSB) += mceusb.o
 obj-$(CONFIG_IR_FINTEK) += fintek-cir.o
 obj-$(CONFIG_IR_MESON) += meson-ir.o
+obj-$(CONFIG_IR_MESON_IRBLASTER) += meson-irblaster.o
 obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
 obj-$(CONFIG_IR_ENE) += ene_ir.o
 obj-$(CONFIG_IR_REDRAT3) += redrat3.o
diff --git a/drivers/media/rc/meson-irblaster.c b/drivers/media/rc/meson-irblaster.c
new file mode 100644
index 000000000000..ef60c8d3dc3e
--- /dev/null
+++ b/drivers/media/rc/meson-irblaster.c
@@ -0,0 +1,433 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/**
+ * meson-irblaster.c - Amlogic Meson IR blaster driver
+ *
+ * Copyright (c) 2021, SberDevices. All Rights Reserved.
+ *
+ * Author: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT. See the GNU General Public License for more
+ * details.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/of_irq.h>
+#include <linux/clk.h>
+#include <media/rc-core.h>
+
+#define DRIVER_NAME	"meson-irblaster"
+
+#define dprintk(x...)	{ if (debug) pr_info(DRIVER_NAME ": " x); }
+
+#define IRB_DEFAULT_CARRIER	38000
+#define IRB_DEFAULT_DUTY_CYCLE	50
+
+#define IRB_FIFO_LEN			128
+#define IRB_DEFAULT_MAX_FIFO_LEVEL	96
+
+#define IRB_ADDR0	0x0
+#define IRB_ADDR1	0x4
+#define IRB_ADDR2	0x8
+#define IRB_ADDR3	0xc
+
+#define IRB_MAX_DELAY	(1 << 10)
+#define IRB_DELAY_MASK	(IRB_MAX_DELAY - 1)
+
+/* IRCTRL_IR_BLASTER_ADDR0 */
+#define IRB_MOD_CLK(x)		((x) << 12)
+#define IRB_MOD_SYS_CLK		0
+#define IRB_MOD_XTAL3_CLK	1
+#define IRB_MOD_1US_CLK		2
+#define IRB_MOD_10US_CLK	3
+#define IRB_INIT_HIGH		BIT(2)
+#define IRB_ENABLE		BIT(0)
+
+/* IRCTRL_IR_BLASTER_ADDR2 */
+#define IRB_MOD_COUNT(lo, hi)	((((lo) - 1) << 16) | ((hi) - 1))
+
+/* IRCTRL_IR_BLASTER_ADDR2 */
+#define IRB_WRITE_FIFO	BIT(16)
+#define IRB_MOD_ENABLE	BIT(12)
+#define IRB_TB_1US	(0x0 << 10)
+#define IRB_TB_10US	(0x1 << 10)
+#define IRB_TB_100US	(0x2 << 10)
+#define IRB_TB_MOD_CLK	(0x3 << 10)
+
+/* IRCTRL_IR_BLASTER_ADDR3 */
+#define IRB_FIFO_THD_PENDING	BIT(16)
+#define IRB_FIFO_IRQ_ENABLE	BIT(8)
+
+static bool debug;
+module_param(debug, bool, 0644);
+MODULE_PARM_DESC(debug, "Enable debug messages");
+
+static unsigned int max_fifo_level = IRB_DEFAULT_MAX_FIFO_LEVEL;
+module_param(max_fifo_level, uint, 0444);
+MODULE_PARM_DESC(max_fifo_level, "Max blaster FIFO filling level");
+
+struct irblaster_dev {
+	unsigned int irq;
+	void __iomem *reg_base;
+	unsigned int *buf;
+	unsigned int buf_len;
+	unsigned int buf_head;
+	unsigned int carrier;
+	unsigned int duty_cycle;
+	spinlock_t lock;
+	struct completion completion;
+	unsigned int max_fifo_level;
+	unsigned int clk_nr;
+	unsigned long clk_rate;
+};
+
+static void irb_set_mod(struct irblaster_dev *irb)
+{
+	unsigned int cnt = irb->clk_rate / irb->carrier;
+	unsigned int pulse_cnt = cnt * irb->duty_cycle / 100;
+	unsigned int space_cnt = cnt - pulse_cnt;
+
+	dprintk("F_mod = %uHz, T_mod = %luns, duty_cycle = %u%%\n",
+		irb->carrier, NSEC_PER_SEC / irb->clk_rate * cnt,
+		100 * pulse_cnt / cnt);
+
+	writel(IRB_MOD_COUNT(pulse_cnt, space_cnt),
+	       irb->reg_base + IRB_ADDR1);
+}
+
+static void irb_setup(struct irblaster_dev *irb)
+{
+	unsigned int fifo_irq_threshold = IRB_FIFO_LEN - irb->max_fifo_level;
+
+	/*
+	 * Disable the blaster, set modulator clock tick and set initialize
+	 * output to be high. Set up carrier frequency and duty cycle. Then
+	 * unset initialize output. Enable FIFO interrupt, set FIFO interrupt
+	 * threshold. Finally, enable the blaster back.
+	 */
+	writel(~IRB_ENABLE & (IRB_MOD_CLK(irb->clk_nr) | IRB_INIT_HIGH),
+	       irb->reg_base + IRB_ADDR0);
+	irb_set_mod(irb);
+	writel(readl(irb->reg_base + IRB_ADDR0) & ~IRB_INIT_HIGH,
+	       irb->reg_base + IRB_ADDR0);
+	writel(IRB_FIFO_IRQ_ENABLE | fifo_irq_threshold,
+	       irb->reg_base + IRB_ADDR3);
+	writel(readl(irb->reg_base + IRB_ADDR0) | IRB_ENABLE,
+	       irb->reg_base + IRB_ADDR0);
+}
+
+static void irb_fifo_push_pulse(struct irblaster_dev *irb, unsigned int time)
+{
+	unsigned int delay;
+	unsigned int tb = IRB_TB_MOD_CLK;
+	unsigned int tb_us = USEC_PER_SEC / irb->carrier;
+
+	delay = (DIV_ROUND_CLOSEST_ULL(time, tb_us) - 1) & IRB_DELAY_MASK;
+	writel((IRB_WRITE_FIFO | IRB_MOD_ENABLE) | tb | delay,
+	       irb->reg_base + IRB_ADDR2);
+}
+
+static void irb_fifo_push_space(struct irblaster_dev *irb, unsigned int time)
+{
+	unsigned int delay;
+	unsigned int tb = IRB_TB_100US;
+	unsigned int tb_us = 100;
+
+	if (time <= IRB_MAX_DELAY) {
+		tb = IRB_TB_1US;
+		tb_us = 1;
+	} else if (time <= 10 * IRB_MAX_DELAY) {
+		tb = IRB_TB_10US;
+		tb_us = 10;
+	} else if (time <= 100 * IRB_MAX_DELAY) {
+		tb = IRB_TB_100US;
+		tb_us = 100;
+	}
+
+	delay = (DIV_ROUND_CLOSEST_ULL(time, tb_us) - 1) & IRB_DELAY_MASK;
+	writel((IRB_WRITE_FIFO & ~IRB_MOD_ENABLE) | tb | delay,
+	       irb->reg_base + IRB_ADDR2);
+}
+
+static void irb_send_buffer(struct irblaster_dev *irb)
+{
+	unsigned long flags;
+	unsigned int nr = 0;
+
+	spin_lock_irqsave(&irb->lock, flags);
+	while (irb->buf_head < irb->buf_len && nr < irb->max_fifo_level) {
+		if (irb->buf_head % 2 == 0)
+			irb_fifo_push_pulse(irb, irb->buf[irb->buf_head]);
+		else
+			irb_fifo_push_space(irb, irb->buf[irb->buf_head]);
+
+		irb->buf_head++;
+		nr++;
+	}
+	spin_unlock_irqrestore(&irb->lock, flags);
+}
+
+static bool irb_check_buf(struct irblaster_dev *irb,
+			  unsigned int *buf, unsigned int len)
+{
+	unsigned int i;
+
+	for (i = 0; i < len; i++) {
+		unsigned int max_tb_us;
+		/*
+		 * Max space timebase is 100 us.
+		 * Pulse timebase equals to carrier period.
+		 */
+		if (i % 2 == 0)
+			max_tb_us = USEC_PER_SEC / irb->carrier;
+		else
+			max_tb_us = 100;
+
+		if (buf[i] >= max_tb_us * IRB_MAX_DELAY)
+			return false;
+	}
+
+	return true;
+}
+
+static void irb_send(struct irblaster_dev *irb,
+		     unsigned int *buf, unsigned int len)
+{
+	reinit_completion(&irb->completion);
+
+	irb->buf = buf;
+	irb->buf_len = len;
+	irb->buf_head = 0;
+
+	dprintk("tx started, buffer length = %u\n", len);
+	irb_send_buffer(irb);
+	wait_for_completion_interruptible(&irb->completion);
+	dprintk("tx completed\n");
+}
+
+static irqreturn_t irb_irqhandler(int irq, void *data)
+{
+	struct irblaster_dev *irb = data;
+
+	writel(readl(irb->reg_base + IRB_ADDR3) & ~IRB_FIFO_THD_PENDING,
+	       irb->reg_base + IRB_ADDR3);
+
+	if (irb->buf_head < irb->buf_len)
+		return IRQ_WAKE_THREAD;
+
+	complete(&irb->completion);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t irb_thread_irqhandler(int irq, void *data)
+{
+	struct irblaster_dev *irb = data;
+
+	irb_send_buffer(irb);
+
+	return IRQ_HANDLED;
+}
+
+static int irb_set_tx_carrier(struct rc_dev *rc, u32 carrier)
+{
+	struct irblaster_dev *irb = rc->priv;
+
+	irb->carrier = carrier;
+	irb_set_mod(irb);
+
+	return 0;
+}
+
+static int irb_set_tx_duty_cycle(struct rc_dev *rc, u32 duty_cycle)
+{
+	struct irblaster_dev *irb = rc->priv;
+
+	irb->duty_cycle = duty_cycle;
+	irb_set_mod(irb);
+
+	return 0;
+}
+
+static int irb_tx_ir(struct rc_dev *rc, unsigned int *buf, unsigned int len)
+{
+	struct irblaster_dev *irb = rc->priv;
+
+	if (!irb_check_buf(irb, buf, len))
+		return -EINVAL;
+
+	irb_send(irb, buf, len);
+
+	return len;
+}
+
+static int irb_mod_clock_probe(struct irblaster_dev *irb, struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct clk *clock;
+	const char *clock_name;
+
+	if (!np)
+		return -ENODEV;
+
+	if (!of_property_read_string(np, "mod-clock", &clock_name)) {
+		if (!strcmp(clock_name, "sysclk"))
+			irb->clk_nr = IRB_MOD_SYS_CLK;
+		else if (!strcmp(clock_name, "xtal"))
+			irb->clk_nr = IRB_MOD_XTAL3_CLK;
+		else
+			return -EINVAL;
+
+		clock = devm_clk_get(dev, clock_name);
+		if (IS_ERR(clock) || clk_prepare_enable(clock))
+			return -ENODEV;
+	} else {
+		irb->clk_nr = IRB_MOD_1US_CLK;
+	}
+
+	switch (irb->clk_nr) {
+	case IRB_MOD_SYS_CLK:
+		irb->clk_rate = clk_get_rate(clock);
+		break;
+	case IRB_MOD_XTAL3_CLK:
+		irb->clk_rate = clk_get_rate(clock) / 3;
+		break;
+	case IRB_MOD_1US_CLK:
+		irb->clk_rate = 1000000;
+		break;
+	}
+
+	dprintk("F_clk = %luHz\n", irb->clk_rate);
+
+	return 0;
+}
+
+static int __init irblaster_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct irblaster_dev *irb;
+	struct rc_dev *rc;
+	struct resource *range;
+	int ret;
+
+	irb = devm_kzalloc(dev, sizeof(*irb), GFP_KERNEL);
+	if (!irb)
+		return -ENOMEM;
+
+	range = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!range) {
+		dev_err(dev, "no memory resource found\n");
+		return -ENODEV;
+	}
+
+	irb->reg_base = devm_ioremap_resource(dev, range);
+	if (IS_ERR(irb->reg_base)) {
+		dev_err(dev, "ioremap failed\n");
+		return PTR_ERR(irb->reg_base);
+	}
+
+	irb->irq = platform_get_irq(pdev, 0);
+	if (irb->irq < 0) {
+		dev_err(dev, "no irq resource found\n");
+		return -ENODEV;
+	}
+
+	if (max_fifo_level <= IRB_FIFO_LEN)
+		irb->max_fifo_level = max_fifo_level;
+	else {
+		irb->max_fifo_level = IRB_FIFO_LEN;
+		dev_warn(dev, "max FIFO level param truncated to %u",
+			 IRB_FIFO_LEN);
+	}
+
+	irb->carrier = IRB_DEFAULT_CARRIER;
+	irb->duty_cycle = IRB_DEFAULT_DUTY_CYCLE;
+	init_completion(&irb->completion);
+	spin_lock_init(&irb->lock);
+
+	ret = irb_mod_clock_probe(irb, dev);
+	if (ret) {
+		dev_err(dev, "modulator clock setup failed\n");
+		return ret;
+	}
+	irb_setup(irb);
+
+	ret = devm_request_threaded_irq(dev, irb->irq,
+					irb_irqhandler,
+					irb_thread_irqhandler,
+					IRQF_TRIGGER_RISING,
+					DRIVER_NAME, irb);
+	if (ret) {
+		dev_err(dev, "irq request failed\n");
+		return ret;
+	}
+
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
+	if (!rc)
+		return -ENOMEM;
+
+	rc->driver_name = DRIVER_NAME;
+	rc->priv = irb;
+
+	rc->tx_ir = irb_tx_ir;
+	rc->s_tx_carrier = irb_set_tx_carrier;
+	rc->s_tx_duty_cycle = irb_set_tx_duty_cycle;
+
+	ret = rc_register_device(rc);
+	if (ret < 0) {
+		dev_err(dev, "rc_dev registration failed\n");
+		rc_free_device(rc);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, rc);
+
+	return 0;
+}
+
+static int irblaster_remove(struct platform_device *pdev)
+{
+	struct rc_dev *rc = platform_get_drvdata(pdev);
+
+	rc_unregister_device(rc);
+
+	return 0;
+}
+
+static const struct of_device_id irblaster_dt_match[] = {
+	{
+		.compatible = "amlogic,meson-irblaster",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, irblaster_dt_match);
+
+static struct platform_driver irblaster_pd = {
+	.remove = irblaster_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.owner  = THIS_MODULE,
+		.of_match_table = irblaster_dt_match,
+	},
+};
+
+module_platform_driver_probe(irblaster_pd, irblaster_probe);
+
+MODULE_DESCRIPTION("Meson IR blaster driver");
+MODULE_AUTHOR("Viktor Prutyanov <viktor.prutyanov@phystech.edu>");
+MODULE_LICENSE("GPL");
-- 
2.21.0

Re: [PATCH 2/2] media: rc: introduce Meson IR blaster driver

[Sean Young]

Hi Viktor,

Thank you for your driver. Is there a datasheet available for this hardware?

On Fri, Jul 02, 2021 at 12:51:32AM +0300, Viktor Prutyanov wrote:
> This patch adds the driver for Amlogic Meson IR blaster.
> 
> Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
> (blaster) controller onboard. It is capable of sending IR
> signals with arbitrary carrier frequency and duty cycle.
> 
> The driver supports 3 modulation clock sources:
>  - sysclk
>  - xtal3 clock (xtal divided by 3)
>  - 1us clock
> 
> Signed-off-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> ---
>  drivers/media/rc/Kconfig           |  10 +
>  drivers/media/rc/Makefile          |   1 +
>  drivers/media/rc/meson-irblaster.c | 433 +++++++++++++++++++++++++++++
>  3 files changed, 444 insertions(+)
>  create mode 100644 drivers/media/rc/meson-irblaster.c
> 
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index d0a8326b75c2..6e60348e1bcf 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -246,6 +246,16 @@ config IR_MESON
>  	   To compile this driver as a module, choose M here: the
>  	   module will be called meson-ir.
>  
> +config IR_MESON_IRBLASTER
> +	tristate "Amlogic Meson IR blaster"
> +	depends on ARCH_MESON || COMPILE_TEST
> +	help
> +	   Say Y if you want to use the IR blaster available on
> +	   Amlogic Meson SoCs.
> +
> +	   To compile this driver as a module, choose M here: the
> +	   module will be called meson-irblaster.
> +
>  config IR_MTK
>  	tristate "Mediatek IR remote receiver"
>  	depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 692e9b6b203f..b108f2b0420c 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_IR_ITE_CIR) += ite-cir.o
>  obj-$(CONFIG_IR_MCEUSB) += mceusb.o
>  obj-$(CONFIG_IR_FINTEK) += fintek-cir.o
>  obj-$(CONFIG_IR_MESON) += meson-ir.o
> +obj-$(CONFIG_IR_MESON_IRBLASTER) += meson-irblaster.o
>  obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
>  obj-$(CONFIG_IR_ENE) += ene_ir.o
>  obj-$(CONFIG_IR_REDRAT3) += redrat3.o
> diff --git a/drivers/media/rc/meson-irblaster.c b/drivers/media/rc/meson-irblaster.c
> new file mode 100644
> index 000000000000..ef60c8d3dc3e
> --- /dev/null
> +++ b/drivers/media/rc/meson-irblaster.c
> @@ -0,0 +1,433 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/**
> + * meson-irblaster.c - Amlogic Meson IR blaster driver
> + *
> + * Copyright (c) 2021, SberDevices. All Rights Reserved.
> + *
> + * Author: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> + *

No need to include the gpl boilerplate as you already have:
// SPDX-License-Identifier: GPL-2.0-only

> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License and no later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/of_irq.h>
> +#include <linux/clk.h>
> +#include <media/rc-core.h>
> +
> +#define DRIVER_NAME	"meson-irblaster"
> +
> +#define dprintk(x...)	{ if (debug) pr_info(DRIVER_NAME ": " x); }

Please use dev_dbg().

> +#define IRB_DEFAULT_CARRIER	38000
> +#define IRB_DEFAULT_DUTY_CYCLE	50
> +
> +#define IRB_FIFO_LEN			128
> +#define IRB_DEFAULT_MAX_FIFO_LEVEL	96
> +
> +#define IRB_ADDR0	0x0
> +#define IRB_ADDR1	0x4
> +#define IRB_ADDR2	0x8
> +#define IRB_ADDR3	0xc
> +
> +#define IRB_MAX_DELAY	(1 << 10)
> +#define IRB_DELAY_MASK	(IRB_MAX_DELAY - 1)
> +
> +/* IRCTRL_IR_BLASTER_ADDR0 */
> +#define IRB_MOD_CLK(x)		((x) << 12)
> +#define IRB_MOD_SYS_CLK		0
> +#define IRB_MOD_XTAL3_CLK	1
> +#define IRB_MOD_1US_CLK		2
> +#define IRB_MOD_10US_CLK	3
> +#define IRB_INIT_HIGH		BIT(2)
> +#define IRB_ENABLE		BIT(0)
> +
> +/* IRCTRL_IR_BLASTER_ADDR2 */
> +#define IRB_MOD_COUNT(lo, hi)	((((lo) - 1) << 16) | ((hi) - 1))
> +
> +/* IRCTRL_IR_BLASTER_ADDR2 */
> +#define IRB_WRITE_FIFO	BIT(16)
> +#define IRB_MOD_ENABLE	BIT(12)
> +#define IRB_TB_1US	(0x0 << 10)
> +#define IRB_TB_10US	(0x1 << 10)
> +#define IRB_TB_100US	(0x2 << 10)
> +#define IRB_TB_MOD_CLK	(0x3 << 10)
> +
> +/* IRCTRL_IR_BLASTER_ADDR3 */
> +#define IRB_FIFO_THD_PENDING	BIT(16)
> +#define IRB_FIFO_IRQ_ENABLE	BIT(8)
> +
> +static bool debug;
> +module_param(debug, bool, 0644);
> +MODULE_PARM_DESC(debug, "Enable debug messages");

With dynamic debug, you don't need this module option.

> +static unsigned int max_fifo_level = IRB_DEFAULT_MAX_FIFO_LEVEL;
> +module_param(max_fifo_level, uint, 0444);
> +MODULE_PARM_DESC(max_fifo_level, "Max blaster FIFO filling level");

Why would you want to lower the fifo size? Is this module parameter ever
needed?

> +
> +struct irblaster_dev {
> +	unsigned int irq;
> +	void __iomem *reg_base;
> +	unsigned int *buf;
> +	unsigned int buf_len;
> +	unsigned int buf_head;
> +	unsigned int carrier;
> +	unsigned int duty_cycle;
> +	spinlock_t lock;
> +	struct completion completion;
> +	unsigned int max_fifo_level;
> +	unsigned int clk_nr;
> +	unsigned long clk_rate;
> +};
> +
> +static void irb_set_mod(struct irblaster_dev *irb)
> +{
> +	unsigned int cnt = irb->clk_rate / irb->carrier;
> +	unsigned int pulse_cnt = cnt * irb->duty_cycle / 100;
> +	unsigned int space_cnt = cnt - pulse_cnt;
> +
> +	dprintk("F_mod = %uHz, T_mod = %luns, duty_cycle = %u%%\n",
> +		irb->carrier, NSEC_PER_SEC / irb->clk_rate * cnt,
> +		100 * pulse_cnt / cnt);

dev_dbg()

> +
> +	writel(IRB_MOD_COUNT(pulse_cnt, space_cnt),
> +	       irb->reg_base + IRB_ADDR1);
> +}
> +
> +static void irb_setup(struct irblaster_dev *irb)
> +{
> +	unsigned int fifo_irq_threshold = IRB_FIFO_LEN - irb->max_fifo_level;
> +
> +	/*
> +	 * Disable the blaster, set modulator clock tick and set initialize
> +	 * output to be high. Set up carrier frequency and duty cycle. Then
> +	 * unset initialize output. Enable FIFO interrupt, set FIFO interrupt
> +	 * threshold. Finally, enable the blaster back.
> +	 */
> +	writel(~IRB_ENABLE & (IRB_MOD_CLK(irb->clk_nr) | IRB_INIT_HIGH),
> +	       irb->reg_base + IRB_ADDR0);
> +	irb_set_mod(irb);
> +	writel(readl(irb->reg_base + IRB_ADDR0) & ~IRB_INIT_HIGH,
> +	       irb->reg_base + IRB_ADDR0);
> +	writel(IRB_FIFO_IRQ_ENABLE | fifo_irq_threshold,
> +	       irb->reg_base + IRB_ADDR3);
> +	writel(readl(irb->reg_base + IRB_ADDR0) | IRB_ENABLE,
> +	       irb->reg_base + IRB_ADDR0);
> +}
> +
> +static void irb_fifo_push_pulse(struct irblaster_dev *irb, unsigned int time)
> +{
> +	unsigned int delay;
> +	unsigned int tb = IRB_TB_MOD_CLK;
> +	unsigned int tb_us = USEC_PER_SEC / irb->carrier;
> +
> +	delay = (DIV_ROUND_CLOSEST_ULL(time, tb_us) - 1) & IRB_DELAY_MASK;
> +	writel((IRB_WRITE_FIFO | IRB_MOD_ENABLE) | tb | delay,
> +	       irb->reg_base + IRB_ADDR2);
> +}
> +
> +static void irb_fifo_push_space(struct irblaster_dev *irb, unsigned int time)
> +{
> +	unsigned int delay;
> +	unsigned int tb = IRB_TB_100US;
> +	unsigned int tb_us = 100;
> +
> +	if (time <= IRB_MAX_DELAY) {
> +		tb = IRB_TB_1US;
> +		tb_us = 1;
> +	} else if (time <= 10 * IRB_MAX_DELAY) {
> +		tb = IRB_TB_10US;
> +		tb_us = 10;
> +	} else if (time <= 100 * IRB_MAX_DELAY) {
> +		tb = IRB_TB_100US;
> +		tb_us = 100;
> +	}
> +
> +	delay = (DIV_ROUND_CLOSEST_ULL(time, tb_us) - 1) & IRB_DELAY_MASK;
> +	writel((IRB_WRITE_FIFO & ~IRB_MOD_ENABLE) | tb | delay,
> +	       irb->reg_base + IRB_ADDR2);
> +}
> +
> +static void irb_send_buffer(struct irblaster_dev *irb)
> +{
> +	unsigned long flags;
> +	unsigned int nr = 0;
> +
> +	spin_lock_irqsave(&irb->lock, flags);
> +	while (irb->buf_head < irb->buf_len && nr < irb->max_fifo_level) {
> +		if (irb->buf_head % 2 == 0)
> +			irb_fifo_push_pulse(irb, irb->buf[irb->buf_head]);
> +		else
> +			irb_fifo_push_space(irb, irb->buf[irb->buf_head]);
> +
> +		irb->buf_head++;
> +		nr++;
> +	}
> +	spin_unlock_irqrestore(&irb->lock, flags);
> +}
> +
> +static bool irb_check_buf(struct irblaster_dev *irb,
> +			  unsigned int *buf, unsigned int len)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < len; i++) {
> +		unsigned int max_tb_us;
> +		/*
> +		 * Max space timebase is 100 us.
> +		 * Pulse timebase equals to carrier period.
> +		 */
> +		if (i % 2 == 0)
> +			max_tb_us = USEC_PER_SEC / irb->carrier;
> +		else
> +			max_tb_us = 100;
> +
> +		if (buf[i] >= max_tb_us * IRB_MAX_DELAY)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void irb_send(struct irblaster_dev *irb,
> +		     unsigned int *buf, unsigned int len)
> +{
> +	reinit_completion(&irb->completion);
> +
> +	irb->buf = buf;
> +	irb->buf_len = len;
> +	irb->buf_head = 0;
> +
> +	dprintk("tx started, buffer length = %u\n", len);
> +	irb_send_buffer(irb);
> +	wait_for_completion_interruptible(&irb->completion);
> +	dprintk("tx completed\n");
> +}
> +
> +static irqreturn_t irb_irqhandler(int irq, void *data)
> +{
> +	struct irblaster_dev *irb = data;
> +
> +	writel(readl(irb->reg_base + IRB_ADDR3) & ~IRB_FIFO_THD_PENDING,
> +	       irb->reg_base + IRB_ADDR3);
> +
> +	if (irb->buf_head < irb->buf_len)
> +		return IRQ_WAKE_THREAD;
> +
> +	complete(&irb->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t irb_thread_irqhandler(int irq, void *data)
> +{
> +	struct irblaster_dev *irb = data;
> +
> +	irb_send_buffer(irb);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int irb_set_tx_carrier(struct rc_dev *rc, u32 carrier)
> +{
> +	struct irblaster_dev *irb = rc->priv;
> +
> +	irb->carrier = carrier;

carrier might be 0 for unmodulated IR. This will make irb_set_mod()
do a division by zero.

Please check appropriate range for carrier and support unmodulated IR
(carrier = 0) if possible.

> +	irb_set_mod(irb);
> +
> +	return 0;
> +}
> +
> +static int irb_set_tx_duty_cycle(struct rc_dev *rc, u32 duty_cycle)
> +{
> +	struct irblaster_dev *irb = rc->priv;
> +
> +	irb->duty_cycle = duty_cycle;
> +	irb_set_mod(irb);
> +
> +	return 0;
> +}
> +
> +static int irb_tx_ir(struct rc_dev *rc, unsigned int *buf, unsigned int len)
> +{
> +	struct irblaster_dev *irb = rc->priv;
> +
> +	if (!irb_check_buf(irb, buf, len))
> +		return -EINVAL;
> +
> +	irb_send(irb, buf, len);
> +
> +	return len;
> +}
> +
> +static int irb_mod_clock_probe(struct irblaster_dev *irb, struct device *dev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct clk *clock;
> +	const char *clock_name;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	if (!of_property_read_string(np, "mod-clock", &clock_name)) {
> +		if (!strcmp(clock_name, "sysclk"))
> +			irb->clk_nr = IRB_MOD_SYS_CLK;
> +		else if (!strcmp(clock_name, "xtal"))
> +			irb->clk_nr = IRB_MOD_XTAL3_CLK;
> +		else
> +			return -EINVAL;
> +
> +		clock = devm_clk_get(dev, clock_name);
> +		if (IS_ERR(clock) || clk_prepare_enable(clock))
> +			return -ENODEV;
> +	} else {
> +		irb->clk_nr = IRB_MOD_1US_CLK;
> +	}
> +
> +	switch (irb->clk_nr) {
> +	case IRB_MOD_SYS_CLK:
> +		irb->clk_rate = clk_get_rate(clock);
> +		break;
> +	case IRB_MOD_XTAL3_CLK:
> +		irb->clk_rate = clk_get_rate(clock) / 3;
> +		break;
> +	case IRB_MOD_1US_CLK:
> +		irb->clk_rate = 1000000;
> +		break;
> +	}
> +
> +	dprintk("F_clk = %luHz\n", irb->clk_rate);
> +
> +	return 0;
> +}
> +
> +static int __init irblaster_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct irblaster_dev *irb;
> +	struct rc_dev *rc;
> +	struct resource *range;
> +	int ret;
> +
> +	irb = devm_kzalloc(dev, sizeof(*irb), GFP_KERNEL);
> +	if (!irb)
> +		return -ENOMEM;
> +
> +	range = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!range) {
> +		dev_err(dev, "no memory resource found\n");
> +		return -ENODEV;
> +	}
> +
> +	irb->reg_base = devm_ioremap_resource(dev, range);
> +	if (IS_ERR(irb->reg_base)) {
> +		dev_err(dev, "ioremap failed\n");
> +		return PTR_ERR(irb->reg_base);
> +	}
> +
> +	irb->irq = platform_get_irq(pdev, 0);
> +	if (irb->irq < 0) {
> +		dev_err(dev, "no irq resource found\n");
> +		return -ENODEV;
> +	}
> +
> +	if (max_fifo_level <= IRB_FIFO_LEN)
> +		irb->max_fifo_level = max_fifo_level;
> +	else {
> +		irb->max_fifo_level = IRB_FIFO_LEN;
> +		dev_warn(dev, "max FIFO level param truncated to %u",
> +			 IRB_FIFO_LEN);
> +	}
> +
> +	irb->carrier = IRB_DEFAULT_CARRIER;
> +	irb->duty_cycle = IRB_DEFAULT_DUTY_CYCLE;
> +	init_completion(&irb->completion);
> +	spin_lock_init(&irb->lock);
> +
> +	ret = irb_mod_clock_probe(irb, dev);
> +	if (ret) {
> +		dev_err(dev, "modulator clock setup failed\n");
> +		return ret;
> +	}
> +	irb_setup(irb);
> +
> +	ret = devm_request_threaded_irq(dev, irb->irq,
> +					irb_irqhandler,
> +					irb_thread_irqhandler,
> +					IRQF_TRIGGER_RISING,
> +					DRIVER_NAME, irb);

Rather than using threaded irqs, would it make more sense to convert
the tx data to the right format before starting tx, thus avoiding doing
expensive(ish) conversions during interrupt handling.

Then the interrupt handler would just need to feed the fifo from a buffer,
which can be done without a threaded irq. Threaded irq might be an issue if
the thread handler does not get called in time and the hardware runs out
of tx data.

> +	if (ret) {
> +		dev_err(dev, "irq request failed\n");
> +		return ret;
> +	}
> +
> +	rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
> +	if (!rc)
> +		return -ENOMEM;
> +
> +	rc->driver_name = DRIVER_NAME;

Please set rc->device_name as well.

> +	rc->priv = irb;
> +
> +	rc->tx_ir = irb_tx_ir;
> +	rc->s_tx_carrier = irb_set_tx_carrier;
> +	rc->s_tx_duty_cycle = irb_set_tx_duty_cycle;
> +
> +	ret = rc_register_device(rc);
> +	if (ret < 0) {
> +		dev_err(dev, "rc_dev registration failed\n");
> +		rc_free_device(rc);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, rc);
> +
> +	return 0;
> +}
> +
> +static int irblaster_remove(struct platform_device *pdev)
> +{
> +	struct rc_dev *rc = platform_get_drvdata(pdev);
> +
> +	rc_unregister_device(rc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id irblaster_dt_match[] = {
> +	{
> +		.compatible = "amlogic,meson-irblaster",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, irblaster_dt_match);
> +
> +static struct platform_driver irblaster_pd = {
> +	.remove = irblaster_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.owner  = THIS_MODULE,
> +		.of_match_table = irblaster_dt_match,
> +	},
> +};
> +
> +module_platform_driver_probe(irblaster_pd, irblaster_probe);
> +
> +MODULE_DESCRIPTION("Meson IR blaster driver");
> +MODULE_AUTHOR("Viktor Prutyanov <viktor.prutyanov@phystech.edu>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.21.0

Thanks,

Sean

Re: [PATCH 2/2] media: rc: introduce Meson IR blaster driver

[kernel test robot]

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

Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.13 next-20210701]
[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/Viktor-Prutyanov/media-rc-add-support-for-Amlogic-Meson-IR-blaster/20210702-055205
base:   git://linuxtv.org/media_tree.git master
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-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/53efbdfa456ffd06aa52fe941890dc1a32963b0b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Viktor-Prutyanov/media-rc-add-support-for-Amlogic-Meson-IR-blaster/20210702-055205
        git checkout 53efbdfa456ffd06aa52fe941890dc1a32963b0b
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/media/rc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/rc/meson-irblaster.c:34: warning: expecting prototype for meson(). Prototype was for DRIVER_NAME() instead


vim +34 drivers/media/rc/meson-irblaster.c

    33	
  > 34	#define DRIVER_NAME	"meson-irblaster"
    35	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68178 bytes --]

Re: [PATCH 1/2] media: rc: meson-irblaster: document device tree bindings

[Martin Blumenstingl]

Hi Viktor,

On Thu, Jul 1, 2021 at 11:51 PM Viktor Prutyanov
<viktor.prutyanov@phystech.edu> wrote:
>
> This patch adds binding documentation for the IR transmitter
> available in Amlogic Meson SoCs.
This is an interesting piece of hardware where I've always wondered if
there is any device out there which supports this functionality.It
turns out that there is

[...]
> +description: |
> +  Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
> +  (blaster) controller onboard. It is capable of sending IR signals
> +  with arbitrary carrier frequency and duty cycle.
> +
> +properties:
> +  compatible:
> +    const: amlogic,meson-irblaster
if you feel like some registers or register values are specific to
A311D or T950D4 then please also add a SoC-specific compatible string
(for example: amlogic,meson-g12b-irblaster).
An example can be seen in
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml

[...]
> +  clocks:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 2
from my understanding there are two clock inputs to the hardware
dt-bindings should always describe the hardware, not what the driver
may (or may not) use.
based on that I think you should drop minItems (then minItems will
have the same value as maxItems)

[...]
> +  mod-clock:
> +    oneOf:
> +      - const: sysclk
> +      - const: xtal
Does this "mod-clock" depend on something external to the IR blaster hardware?
If not this should be handled inside the driver only.

From how I understand the register description in the datasheet
there's two clock inputs.
XTAL is internally divided further down with fixed dividers.
Then there's a configurable divider which is then used to generate the
IR signal.
If the sysclk (I assume that this is clk81 - or at least derived from
it) is "too fast" then the driver should just ignore that clock while
the dt-bindings should still describe it (see my comment above)

[...]
> +    meson-irblaster@ff80014c {
node names should be generic, see for example
Documentation/devicetree/bindings/spi/amlogic,meson6-spifc.yaml
(spifc is the name Amlogic has given this IP, but since node names are
supposed to be generic we use spi@...)

However, I am not sure if an IR blaster would be described as
ir-blaster@... or simply ir@...

> +      compatible = "amlogic,meson-irblaster";
> +      reg = <0xff80014c 0x10>;
> +      interrupts = <0 198 IRQ_TYPE_EDGE_RISING>;
> +      clocks = <&clkc CLKID_CLK81 &xtal>;
[...]
> +      clocks = <&clkc CLKID_CLK81 &xtal>;
while this works I think the recommended format is:
    clocks = <&clkc CLKID_CLK81>, <&xtal>


Best regards,
Martin

Re: [PATCH 1/2] media: rc: meson-irblaster: document device tree bindings

[Neil Armstrong]

Hi,

On 02/07/2021 15:30, Martin Blumenstingl wrote:
> Hi Viktor,
> 
> On Thu, Jul 1, 2021 at 11:51 PM Viktor Prutyanov
> <viktor.prutyanov@phystech.edu> wrote:
>>
>> This patch adds binding documentation for the IR transmitter
>> available in Amlogic Meson SoCs.
> This is an interesting piece of hardware where I've always wondered if
> there is any device out there which supports this functionality.It
> turns out that there is

You did beat me, I started a driver some time ago but failed to finish debugging it...
https://github.com/superna9999/linux/tree/amlogic/v5.2%2Fir-blaster

> 
> [...]
>> +description: |
>> +  Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
>> +  (blaster) controller onboard. It is capable of sending IR signals
>> +  with arbitrary carrier frequency and duty cycle.
>> +
>> +properties:
>> +  compatible:
>> +    const: amlogic,meson-irblaster
> if you feel like some registers or register values are specific to
> A311D or T950D4 then please also add a SoC-specific compatible string
> (for example: amlogic,meson-g12b-irblaster).
> An example can be seen in
> Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml

AFAIK there is 2 versions of the IP, the "old" one we can find on Meson6, 8/8b, GXBB, GXL & GXM (and maybe AXG ?),
and the one we find on the latest G12A, G12B & SM1.

The SEI510 and SEI610 boards we use for Yukawa android port do have the necessary HW for IR sending,
so I'll eventually be able to test.

So, as martin says you should add a "amlogic,g12a-ir-blaster" to be sure we support the older ir blaster version
correctly with the right bindings.

Neil

> 
> [...]
>> +  clocks:
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    minItems: 1
>> +    maxItems: 2
> from my understanding there are two clock inputs to the hardware
> dt-bindings should always describe the hardware, not what the driver
> may (or may not) use.
> based on that I think you should drop minItems (then minItems will
> have the same value as maxItems)
> 
> [...]
>> +  mod-clock:
>> +    oneOf:
>> +      - const: sysclk
>> +      - const: xtal
> Does this "mod-clock" depend on something external to the IR blaster hardware?
> If not this should be handled inside the driver only.
> 
> From how I understand the register description in the datasheet
> there's two clock inputs.
> XTAL is internally divided further down with fixed dividers.
> Then there's a configurable divider which is then used to generate the
> IR signal.
> If the sysclk (I assume that this is clk81 - or at least derived from
> it) is "too fast" then the driver should just ignore that clock while
> the dt-bindings should still describe it (see my comment above)
> 
> [...]
>> +    meson-irblaster@ff80014c {
> node names should be generic, see for example
> Documentation/devicetree/bindings/spi/amlogic,meson6-spifc.yaml
> (spifc is the name Amlogic has given this IP, but since node names are
> supposed to be generic we use spi@...)
> 
> However, I am not sure if an IR blaster would be described as
> ir-blaster@... or simply ir@...
> 
>> +      compatible = "amlogic,meson-irblaster";
>> +      reg = <0xff80014c 0x10>;
>> +      interrupts = <0 198 IRQ_TYPE_EDGE_RISING>;
>> +      clocks = <&clkc CLKID_CLK81 &xtal>;
> [...]
>> +      clocks = <&clkc CLKID_CLK81 &xtal>;
> while this works I think the recommended format is:
>     clocks = <&clkc CLKID_CLK81>, <&xtal>
> 
> 
> Best regards,
> Martin
> 

Re: [PATCH 2/2] media: rc: introduce Meson IR blaster driver

[Martin Blumenstingl]

Hi Sean,

On Fri, Jul 2, 2021 at 12:46 AM Sean Young <sean@mess.org> wrote:
>
> Hi Viktor,
>
> Thank you for your driver. Is there a datasheet available for this hardware?
The public S905X datasheet [0] (starting at page 515) and the public
S905D3 datasheet [1] (starting at page 1105) document the registers.
If Viktor has additional or better information then it would be great
if he could share it with us.


Best regards,
Martin


[0] https://dl.khadas.com/Hardware/VIM1/Datasheet/S905X_Datasheet%20V0.3%2020170314publicversion-Wesion.pdf
[1] https://dl.khadas.com/Hardware/VIM3/Datasheet/S905D3_datasheet_0.2_Wesion.pdf

Re: [PATCH 2/2] media: rc: introduce Meson IR blaster driver

[Viktor Prutyanov]

Hi Martin,

On Fri, 2 Jul 2021 18:15:18 +0200
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Sean,
> 
> On Fri, Jul 2, 2021 at 12:46 AM Sean Young <sean@mess.org> wrote:
> >
> > Hi Viktor,
> >
> > Thank you for your driver. Is there a datasheet available for this
> > hardware?
> The public S905X datasheet [0] (starting at page 515) and the public
> S905D3 datasheet [1] (starting at page 1105) document the registers.
> If Viktor has additional or better information then it would be great
> if he could share it with us.

I can add that descriptions of A311D and T950D4 blasters are the same
as S905D3, including 0xFF800000 base address.
The A311D public datasheet doesn't say anything about IR blaster, but
it is still present on this SoC.

Best regards,
Viktor Prutyanov

Re: [PATCH 2/2] media: rc: introduce Meson IR blaster driver

[kernel test robot]

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

Hi Viktor,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v5.13 next-20210701]
[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/Viktor-Prutyanov/media-rc-add-support-for-Amlogic-Meson-IR-blaster/20210702-055205
base:   git://linuxtv.org/media_tree.git master
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-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/53efbdfa456ffd06aa52fe941890dc1a32963b0b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Viktor-Prutyanov/media-rc-add-support-for-Amlogic-Meson-IR-blaster/20210702-055205
        git checkout 53efbdfa456ffd06aa52fe941890dc1a32963b0b
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: drivers/media/rc/meson-irblaster.o: in function `irb_send_buffer':
>> meson-irblaster.c:(.text+0x418): undefined reference to `__aeabi_uldivmod'

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 78575 bytes --]

Re: [PATCH 2/2] media: rc: introduce Meson IR blaster driver

[Viktor Prutyanov]

Hi Sean,

Thank you for the review, I tried to fix issues your found in the 2nd
version, FIFO watermark parameter. Explanation is below.

On Thu, 1 Jul 2021 23:46:46 +0100
Sean Young <sean@mess.org> wrote:

> Hi Viktor,
> 
> Thank you for your driver. Is there a datasheet available for this
> hardware?
> 
> On Fri, Jul 02, 2021 at 12:51:32AM +0300, Viktor Prutyanov wrote:
> > This patch adds the driver for Amlogic Meson IR blaster.
> > 
> > Some Amlogic SoCs such as A311D and T950D4 have IR transmitter
> > (blaster) controller onboard. It is capable of sending IR
> > signals with arbitrary carrier frequency and duty cycle.
> > 
> > The driver supports 3 modulation clock sources:
> >  - sysclk
> >  - xtal3 clock (xtal divided by 3)
> >  - 1us clock
> > 
> > Signed-off-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> > ---
> >  drivers/media/rc/Kconfig           |  10 +
> >  drivers/media/rc/Makefile          |   1 +
> >  drivers/media/rc/meson-irblaster.c | 433
> > +++++++++++++++++++++++++++++ 3 files changed, 444 insertions(+)
> >  create mode 100644 drivers/media/rc/meson-irblaster.c
> > 
> > diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> > index d0a8326b75c2..6e60348e1bcf 100644
> > --- a/drivers/media/rc/Kconfig
> > +++ b/drivers/media/rc/Kconfig
> > @@ -246,6 +246,16 @@ config IR_MESON
> >  	   To compile this driver as a module, choose M here: the
> >  	   module will be called meson-ir.
> >  
> > +config IR_MESON_IRBLASTER
> > +	tristate "Amlogic Meson IR blaster"
> > +	depends on ARCH_MESON || COMPILE_TEST
> > +	help
> > +	   Say Y if you want to use the IR blaster available on
> > +	   Amlogic Meson SoCs.
> > +
> > +	   To compile this driver as a module, choose M here: the
> > +	   module will be called meson-irblaster.
> > +
> >  config IR_MTK
> >  	tristate "Mediatek IR remote receiver"
> >  	depends on ARCH_MEDIATEK || COMPILE_TEST
> > diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> > index 692e9b6b203f..b108f2b0420c 100644
> > --- a/drivers/media/rc/Makefile
> > +++ b/drivers/media/rc/Makefile
> > @@ -28,6 +28,7 @@ obj-$(CONFIG_IR_ITE_CIR) += ite-cir.o
> >  obj-$(CONFIG_IR_MCEUSB) += mceusb.o
> >  obj-$(CONFIG_IR_FINTEK) += fintek-cir.o
> >  obj-$(CONFIG_IR_MESON) += meson-ir.o
> > +obj-$(CONFIG_IR_MESON_IRBLASTER) += meson-irblaster.o
> >  obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
> >  obj-$(CONFIG_IR_ENE) += ene_ir.o
> >  obj-$(CONFIG_IR_REDRAT3) += redrat3.o
> > diff --git a/drivers/media/rc/meson-irblaster.c
> > b/drivers/media/rc/meson-irblaster.c new file mode 100644
> > index 000000000000..ef60c8d3dc3e
> > --- /dev/null
> > +++ b/drivers/media/rc/meson-irblaster.c
> > @@ -0,0 +1,433 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/**
> > + * meson-irblaster.c - Amlogic Meson IR blaster driver
> > + *
> > + * Copyright (c) 2021, SberDevices. All Rights Reserved.
> > + *
> > + * Author: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> > + *  
> 
> No need to include the gpl boilerplate as you already have:
> // SPDX-License-Identifier: GPL-2.0-only
>
> > + * This program is free software; you can redistribute it and/or
> > modify it
> > + * under the terms of the GNU General Public License as published
> > by the
> > + * Free Software Foundation; version 2 of the License and no later
> > version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE
> > or
> > + * NON INFRINGEMENT. See the GNU General Public License for more
> > + * details.
> > + *
> > + * The full GNU General Public License is included in this
> > distribution in
> > + * the file called "COPYING".
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/clk.h>
> > +#include <media/rc-core.h>
> > +
> > +#define DRIVER_NAME	"meson-irblaster"
> > +
> > +#define dprintk(x...)	{ if (debug) pr_info(DRIVER_NAME ": "
> > x); }  
> 
> Please use dev_dbg().
> 
> > +#define IRB_DEFAULT_CARRIER	38000
> > +#define IRB_DEFAULT_DUTY_CYCLE	50
> > +
> > +#define IRB_FIFO_LEN			128
> > +#define IRB_DEFAULT_MAX_FIFO_LEVEL	96
> > +
> > +#define IRB_ADDR0	0x0
> > +#define IRB_ADDR1	0x4
> > +#define IRB_ADDR2	0x8
> > +#define IRB_ADDR3	0xc
> > +
> > +#define IRB_MAX_DELAY	(1 << 10)
> > +#define IRB_DELAY_MASK	(IRB_MAX_DELAY - 1)
> > +
> > +/* IRCTRL_IR_BLASTER_ADDR0 */
> > +#define IRB_MOD_CLK(x)		((x) << 12)
> > +#define IRB_MOD_SYS_CLK		0
> > +#define IRB_MOD_XTAL3_CLK	1
> > +#define IRB_MOD_1US_CLK		2
> > +#define IRB_MOD_10US_CLK	3
> > +#define IRB_INIT_HIGH		BIT(2)
> > +#define IRB_ENABLE		BIT(0)
> > +
> > +/* IRCTRL_IR_BLASTER_ADDR2 */
> > +#define IRB_MOD_COUNT(lo, hi)	((((lo) - 1) << 16) | ((hi) -
> > 1)) +
> > +/* IRCTRL_IR_BLASTER_ADDR2 */
> > +#define IRB_WRITE_FIFO	BIT(16)
> > +#define IRB_MOD_ENABLE	BIT(12)
> > +#define IRB_TB_1US	(0x0 << 10)
> > +#define IRB_TB_10US	(0x1 << 10)
> > +#define IRB_TB_100US	(0x2 << 10)
> > +#define IRB_TB_MOD_CLK	(0x3 << 10)
> > +
> > +/* IRCTRL_IR_BLASTER_ADDR3 */
> > +#define IRB_FIFO_THD_PENDING	BIT(16)
> > +#define IRB_FIFO_IRQ_ENABLE	BIT(8)
> > +
> > +static bool debug;
> > +module_param(debug, bool, 0644);
> > +MODULE_PARM_DESC(debug, "Enable debug messages");  
> 
> With dynamic debug, you don't need this module option.
> 
> > +static unsigned int max_fifo_level = IRB_DEFAULT_MAX_FIFO_LEVEL;
> > +module_param(max_fifo_level, uint, 0444);
> > +MODULE_PARM_DESC(max_fifo_level, "Max blaster FIFO filling
> > level");  
> 
> Why would you want to lower the fifo size? Is this module parameter
> ever needed?

The idea is following. FIFO size is 128 entries. Interrupt appears when
FIFO IRQ threshold is passed. If we set the threshold to 0, IRQ appears
right after the FIFO becomes empty. It means that IR blaster do nothing
while we pushing next entries. But for example if we set threshold to 24
= 128 - 96, IRQ appears when 24 entries are about to transmit and we
have enough time to push new entries. 

Of course, it has more sense in previous version with threaded IRQ, but
I think it still OK to have time reserve to push entries with a large
number.

> 
> > +
> > +struct irblaster_dev {
> > +	unsigned int irq;
> > +	void __iomem *reg_base;
> > +	unsigned int *buf;
> > +	unsigned int buf_len;
> > +	unsigned int buf_head;
> > +	unsigned int carrier;
> > +	unsigned int duty_cycle;
> > +	spinlock_t lock;
> > +	struct completion completion;
> > +	unsigned int max_fifo_level;
> > +	unsigned int clk_nr;
> > +	unsigned long clk_rate;
> > +};
> > +
> > +static void irb_set_mod(struct irblaster_dev *irb)
> > +{
> > +	unsigned int cnt = irb->clk_rate / irb->carrier;
> > +	unsigned int pulse_cnt = cnt * irb->duty_cycle / 100;
> > +	unsigned int space_cnt = cnt - pulse_cnt;
> > +
> > +	dprintk("F_mod = %uHz, T_mod = %luns, duty_cycle = %u%%\n",
> > +		irb->carrier, NSEC_PER_SEC / irb->clk_rate * cnt,
> > +		100 * pulse_cnt / cnt);  
> 
> dev_dbg()
> 
> > +
> > +	writel(IRB_MOD_COUNT(pulse_cnt, space_cnt),
> > +	       irb->reg_base + IRB_ADDR1);
> > +}
> > +
> > +static void irb_setup(struct irblaster_dev *irb)
> > +{
> > +	unsigned int fifo_irq_threshold = IRB_FIFO_LEN -
> > irb->max_fifo_level; +
> > +	/*
> > +	 * Disable the blaster, set modulator clock tick and set
> > initialize
> > +	 * output to be high. Set up carrier frequency and duty
> > cycle. Then
> > +	 * unset initialize output. Enable FIFO interrupt, set
> > FIFO interrupt
> > +	 * threshold. Finally, enable the blaster back.
> > +	 */
> > +	writel(~IRB_ENABLE & (IRB_MOD_CLK(irb->clk_nr) |
> > IRB_INIT_HIGH),
> > +	       irb->reg_base + IRB_ADDR0);
> > +	irb_set_mod(irb);
> > +	writel(readl(irb->reg_base + IRB_ADDR0) & ~IRB_INIT_HIGH,
> > +	       irb->reg_base + IRB_ADDR0);
> > +	writel(IRB_FIFO_IRQ_ENABLE | fifo_irq_threshold,
> > +	       irb->reg_base + IRB_ADDR3);
> > +	writel(readl(irb->reg_base + IRB_ADDR0) | IRB_ENABLE,
> > +	       irb->reg_base + IRB_ADDR0);
> > +}
> > +
> > +static void irb_fifo_push_pulse(struct irblaster_dev *irb,
> > unsigned int time) +{
> > +	unsigned int delay;
> > +	unsigned int tb = IRB_TB_MOD_CLK;
> > +	unsigned int tb_us = USEC_PER_SEC / irb->carrier;
> > +
> > +	delay = (DIV_ROUND_CLOSEST_ULL(time, tb_us) - 1) &
> > IRB_DELAY_MASK;
> > +	writel((IRB_WRITE_FIFO | IRB_MOD_ENABLE) | tb | delay,
> > +	       irb->reg_base + IRB_ADDR2);
> > +}
> > +
> > +static void irb_fifo_push_space(struct irblaster_dev *irb,
> > unsigned int time) +{
> > +	unsigned int delay;
> > +	unsigned int tb = IRB_TB_100US;
> > +	unsigned int tb_us = 100;
> > +
> > +	if (time <= IRB_MAX_DELAY) {
> > +		tb = IRB_TB_1US;
> > +		tb_us = 1;
> > +	} else if (time <= 10 * IRB_MAX_DELAY) {
> > +		tb = IRB_TB_10US;
> > +		tb_us = 10;
> > +	} else if (time <= 100 * IRB_MAX_DELAY) {
> > +		tb = IRB_TB_100US;
> > +		tb_us = 100;
> > +	}
> > +
> > +	delay = (DIV_ROUND_CLOSEST_ULL(time, tb_us) - 1) &
> > IRB_DELAY_MASK;
> > +	writel((IRB_WRITE_FIFO & ~IRB_MOD_ENABLE) | tb | delay,
> > +	       irb->reg_base + IRB_ADDR2);
> > +}
> > +
> > +static void irb_send_buffer(struct irblaster_dev *irb)
> > +{
> > +	unsigned long flags;
> > +	unsigned int nr = 0;
> > +
> > +	spin_lock_irqsave(&irb->lock, flags);
> > +	while (irb->buf_head < irb->buf_len && nr <
> > irb->max_fifo_level) {
> > +		if (irb->buf_head % 2 == 0)
> > +			irb_fifo_push_pulse(irb,
> > irb->buf[irb->buf_head]);
> > +		else
> > +			irb_fifo_push_space(irb,
> > irb->buf[irb->buf_head]); +
> > +		irb->buf_head++;
> > +		nr++;
> > +	}
> > +	spin_unlock_irqrestore(&irb->lock, flags);
> > +}
> > +
> > +static bool irb_check_buf(struct irblaster_dev *irb,
> > +			  unsigned int *buf, unsigned int len)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < len; i++) {
> > +		unsigned int max_tb_us;
> > +		/*
> > +		 * Max space timebase is 100 us.
> > +		 * Pulse timebase equals to carrier period.
> > +		 */
> > +		if (i % 2 == 0)
> > +			max_tb_us = USEC_PER_SEC / irb->carrier;
> > +		else
> > +			max_tb_us = 100;
> > +
> > +		if (buf[i] >= max_tb_us * IRB_MAX_DELAY)
> > +			return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static void irb_send(struct irblaster_dev *irb,
> > +		     unsigned int *buf, unsigned int len)
> > +{
> > +	reinit_completion(&irb->completion);
> > +
> > +	irb->buf = buf;
> > +	irb->buf_len = len;
> > +	irb->buf_head = 0;
> > +
> > +	dprintk("tx started, buffer length = %u\n", len);
> > +	irb_send_buffer(irb);
> > +	wait_for_completion_interruptible(&irb->completion);
> > +	dprintk("tx completed\n");
> > +}
> > +
> > +static irqreturn_t irb_irqhandler(int irq, void *data)
> > +{
> > +	struct irblaster_dev *irb = data;
> > +
> > +	writel(readl(irb->reg_base + IRB_ADDR3) &
> > ~IRB_FIFO_THD_PENDING,
> > +	       irb->reg_base + IRB_ADDR3);
> > +
> > +	if (irb->buf_head < irb->buf_len)
> > +		return IRQ_WAKE_THREAD;
> > +
> > +	complete(&irb->completion);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t irb_thread_irqhandler(int irq, void *data)
> > +{
> > +	struct irblaster_dev *irb = data;
> > +
> > +	irb_send_buffer(irb);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int irb_set_tx_carrier(struct rc_dev *rc, u32 carrier)
> > +{
> > +	struct irblaster_dev *irb = rc->priv;
> > +
> > +	irb->carrier = carrier;  
> 
> carrier might be 0 for unmodulated IR. This will make irb_set_mod()
> do a division by zero.
> 
> Please check appropriate range for carrier and support unmodulated IR
> (carrier = 0) if possible.
> 
> > +	irb_set_mod(irb);
> > +
> > +	return 0;
> > +}
> > +
> > +static int irb_set_tx_duty_cycle(struct rc_dev *rc, u32 duty_cycle)
> > +{
> > +	struct irblaster_dev *irb = rc->priv;
> > +
> > +	irb->duty_cycle = duty_cycle;
> > +	irb_set_mod(irb);
> > +
> > +	return 0;
> > +}
> > +
> > +static int irb_tx_ir(struct rc_dev *rc, unsigned int *buf,
> > unsigned int len) +{
> > +	struct irblaster_dev *irb = rc->priv;
> > +
> > +	if (!irb_check_buf(irb, buf, len))
> > +		return -EINVAL;
> > +
> > +	irb_send(irb, buf, len);
> > +
> > +	return len;
> > +}
> > +
> > +static int irb_mod_clock_probe(struct irblaster_dev *irb, struct
> > device *dev) +{
> > +	struct device_node *np = dev->of_node;
> > +	struct clk *clock;
> > +	const char *clock_name;
> > +
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	if (!of_property_read_string(np, "mod-clock",
> > &clock_name)) {
> > +		if (!strcmp(clock_name, "sysclk"))
> > +			irb->clk_nr = IRB_MOD_SYS_CLK;
> > +		else if (!strcmp(clock_name, "xtal"))
> > +			irb->clk_nr = IRB_MOD_XTAL3_CLK;
> > +		else
> > +			return -EINVAL;
> > +
> > +		clock = devm_clk_get(dev, clock_name);
> > +		if (IS_ERR(clock) || clk_prepare_enable(clock))
> > +			return -ENODEV;
> > +	} else {
> > +		irb->clk_nr = IRB_MOD_1US_CLK;
> > +	}
> > +
> > +	switch (irb->clk_nr) {
> > +	case IRB_MOD_SYS_CLK:
> > +		irb->clk_rate = clk_get_rate(clock);
> > +		break;
> > +	case IRB_MOD_XTAL3_CLK:
> > +		irb->clk_rate = clk_get_rate(clock) / 3;
> > +		break;
> > +	case IRB_MOD_1US_CLK:
> > +		irb->clk_rate = 1000000;
> > +		break;
> > +	}
> > +
> > +	dprintk("F_clk = %luHz\n", irb->clk_rate);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init irblaster_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct irblaster_dev *irb;
> > +	struct rc_dev *rc;
> > +	struct resource *range;
> > +	int ret;
> > +
> > +	irb = devm_kzalloc(dev, sizeof(*irb), GFP_KERNEL);
> > +	if (!irb)
> > +		return -ENOMEM;
> > +
> > +	range = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!range) {
> > +		dev_err(dev, "no memory resource found\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	irb->reg_base = devm_ioremap_resource(dev, range);
> > +	if (IS_ERR(irb->reg_base)) {
> > +		dev_err(dev, "ioremap failed\n");
> > +		return PTR_ERR(irb->reg_base);
> > +	}
> > +
> > +	irb->irq = platform_get_irq(pdev, 0);
> > +	if (irb->irq < 0) {
> > +		dev_err(dev, "no irq resource found\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (max_fifo_level <= IRB_FIFO_LEN)
> > +		irb->max_fifo_level = max_fifo_level;
> > +	else {
> > +		irb->max_fifo_level = IRB_FIFO_LEN;
> > +		dev_warn(dev, "max FIFO level param truncated to
> > %u",
> > +			 IRB_FIFO_LEN);
> > +	}
> > +
> > +	irb->carrier = IRB_DEFAULT_CARRIER;
> > +	irb->duty_cycle = IRB_DEFAULT_DUTY_CYCLE;
> > +	init_completion(&irb->completion);
> > +	spin_lock_init(&irb->lock);
> > +
> > +	ret = irb_mod_clock_probe(irb, dev);
> > +	if (ret) {
> > +		dev_err(dev, "modulator clock setup failed\n");
> > +		return ret;
> > +	}
> > +	irb_setup(irb);
> > +
> > +	ret = devm_request_threaded_irq(dev, irb->irq,
> > +					irb_irqhandler,
> > +					irb_thread_irqhandler,
> > +					IRQF_TRIGGER_RISING,
> > +					DRIVER_NAME, irb);  
> 
> Rather than using threaded irqs, would it make more sense to convert
> the tx data to the right format before starting tx, thus avoiding
> doing expensive(ish) conversions during interrupt handling.
> 
> Then the interrupt handler would just need to feed the fifo from a
> buffer, which can be done without a threaded irq. Threaded irq might
> be an issue if the thread handler does not get called in time and the
> hardware runs out of tx data.
> 
> > +	if (ret) {
> > +		dev_err(dev, "irq request failed\n");
> > +		return ret;
> > +	}
> > +
> > +	rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
> > +	if (!rc)
> > +		return -ENOMEM;
> > +
> > +	rc->driver_name = DRIVER_NAME;  
> 
> Please set rc->device_name as well.
> 
> > +	rc->priv = irb;
> > +
> > +	rc->tx_ir = irb_tx_ir;
> > +	rc->s_tx_carrier = irb_set_tx_carrier;
> > +	rc->s_tx_duty_cycle = irb_set_tx_duty_cycle;
> > +
> > +	ret = rc_register_device(rc);
> > +	if (ret < 0) {
> > +		dev_err(dev, "rc_dev registration failed\n");
> > +		rc_free_device(rc);
> > +		return ret;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, rc);
> > +
> > +	return 0;
> > +}
> > +
> > +static int irblaster_remove(struct platform_device *pdev)
> > +{
> > +	struct rc_dev *rc = platform_get_drvdata(pdev);
> > +
> > +	rc_unregister_device(rc);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id irblaster_dt_match[] = {
> > +	{
> > +		.compatible = "amlogic,meson-irblaster",
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, irblaster_dt_match);
> > +
> > +static struct platform_driver irblaster_pd = {
> > +	.remove = irblaster_remove,
> > +	.driver = {
> > +		.name = DRIVER_NAME,
> > +		.owner  = THIS_MODULE,
> > +		.of_match_table = irblaster_dt_match,
> > +	},
> > +};
> > +
> > +module_platform_driver_probe(irblaster_pd, irblaster_probe);
> > +
> > +MODULE_DESCRIPTION("Meson IR blaster driver");
> > +MODULE_AUTHOR("Viktor Prutyanov <viktor.prutyanov@phystech.edu>");
> > +MODULE_LICENSE("GPL");
> > -- 
> > 2.21.0  
> 
> Thanks,
> 
> Sean

Best regards,
Viktor Prutyanov