[PATCH 00/11] media: rc: ImgTec IR decoder driver

[PATCH 00/11] media: rc: ImgTec IR decoder driver

[James Hogan]

Add driver for the ImgTec Infrared decoder block. Two separate rc input
devices are exposed depending on kernel configuration. One uses the
hardware decoder which is set up with timings for a specific protocol
and supports mask/value filtering and wake events. The other uses raw
edge interrupts and the generic software protocol decoders to allow
multiple protocols to be supported, including those not supported by the
hardware decoder.

The hardware decoder timing values, raw data to scan code conversion
function and scan code filter to raw data filter conversion function are
provided as separate modules for each protocol which the main driver can
use. The scan code filter value and mask (and the same again for wake
from sleep) are specified via sysfs files in /sys/class/rc/rcX/.

Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: linux-media@vger.kernel.org
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: devicetree@vger.kernel.org

James Hogan (11):
  dt: binding: add binding for ImgTec IR block
  media: rc: img-ir: add base driver
  media: rc: img-ir: add raw driver
  media: rc: img-ir: add hardware decoder driver
  media: rc: img-ir: add to build
  media: rc: img-ir: add NEC decoder module
  media: rc: img-ir: add JVC decoder module
  media: rc: img-ir: add Sony decoder module
  media: rc: add Sharp infrared protocol
  media: rc: img-ir: add Sharp decoder module
  media: rc: img-ir: add Sanyo decoder module

 Documentation/devicetree/bindings/media/img-ir.txt |   20 +
 drivers/media/rc/Kconfig                           |    2 +
 drivers/media/rc/Makefile                          |    1 +
 drivers/media/rc/img-ir/Kconfig                    |   61 +
 drivers/media/rc/img-ir/Makefile                   |   11 +
 drivers/media/rc/img-ir/img-ir-core.c              |  172 +++
 drivers/media/rc/img-ir/img-ir-hw.c                | 1277 ++++++++++++++++++++
 drivers/media/rc/img-ir/img-ir-hw.h                |  284 +++++
 drivers/media/rc/img-ir/img-ir-jvc.c               |  109 ++
 drivers/media/rc/img-ir/img-ir-nec.c               |  149 +++
 drivers/media/rc/img-ir/img-ir-raw.c               |  107 ++
 drivers/media/rc/img-ir/img-ir-raw.h               |   58 +
 drivers/media/rc/img-ir/img-ir-sanyo.c             |  139 +++
 drivers/media/rc/img-ir/img-ir-sharp.c             |  115 ++
 drivers/media/rc/img-ir/img-ir-sony.c              |  163 +++
 drivers/media/rc/img-ir/img-ir.h                   |  170 +++
 drivers/media/rc/rc-main.c                         |    1 +
 include/media/rc-map.h                             |    4 +-
 18 files changed, 2842 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/media/img-ir.txt
 create mode 100644 drivers/media/rc/img-ir/Kconfig
 create mode 100644 drivers/media/rc/img-ir/Makefile
 create mode 100644 drivers/media/rc/img-ir/img-ir-core.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-hw.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-hw.h
 create mode 100644 drivers/media/rc/img-ir/img-ir-jvc.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-nec.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-raw.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-raw.h
 create mode 100644 drivers/media/rc/img-ir/img-ir-sanyo.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-sharp.c
 create mode 100644 drivers/media/rc/img-ir/img-ir-sony.c
 create mode 100644 drivers/media/rc/img-ir/img-ir.h

-- 
1.8.1.2

[PATCH 01/11] dt: binding: add binding for ImgTec IR block

[James Hogan]

Add device tree binding for ImgTec Consumer Infrared block.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: linux-media@vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Rob Landley <rob@landley.net>
Cc: linux-doc@vger.kernel.org
---
 Documentation/devicetree/bindings/media/img-ir.txt | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/img-ir.txt

diff --git a/Documentation/devicetree/bindings/media/img-ir.txt b/Documentation/devicetree/bindings/media/img-ir.txt
new file mode 100644
index 000000000000..6f623b094ea6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/img-ir.txt
@@ -0,0 +1,20 @@
+* ImgTec Infrared (IR) decoder
+
+Required properties:
+- compatible:		Should be "img,ir"
+- reg:			Physical base address of the controller and length of
+			memory mapped region.
+- interrupts:		The interrupt specifier to the cpu.
+
+Optional properties:
+- clocks:		Clock specifier for base clock.
+			Defaults to 32.768KHz if not specified.
+
+Example:
+
+	ir@02006200 {
+		compatible = "img,ir";
+		reg = <0x02006200 0x100>;
+		interrupts = <29 4>;
+		clocks = <&clk_32khz>;
+	};
-- 
1.8.1.2

[PATCH 02/11] media: rc: img-ir: add base driver

[James Hogan]

Add base driver for the ImgTec Infrared decoder block. The driver is
split into separate components for raw (software) decode and hardware
decoder which are in following commits.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: linux-media@vger.kernel.org
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: devicetree@vger.kernel.org
---
 drivers/media/rc/img-ir/img-ir-core.c | 172 ++++++++++++++++++++++++++++++++++
 drivers/media/rc/img-ir/img-ir.h      | 170 +++++++++++++++++++++++++++++++++
 2 files changed, 342 insertions(+)
 create mode 100644 drivers/media/rc/img-ir/img-ir-core.c
 create mode 100644 drivers/media/rc/img-ir/img-ir.h

diff --git a/drivers/media/rc/img-ir/img-ir-core.c b/drivers/media/rc/img-ir/img-ir-core.c
new file mode 100644
index 000000000000..a577217aa739
--- /dev/null
+++ b/drivers/media/rc/img-ir/img-ir-core.c
@@ -0,0 +1,172 @@
+/*
+ * ImgTec IR Decoder found in PowerDown Controller.
+ *
+ * Copyright 2010-2013 Imagination Technologies Ltd.
+ *
+ * This contains core img-ir code for setting up the driver. The two interfaces
+ * (raw and hardware decode) are handled separately.
+ */
+
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include "img-ir.h"
+
+static irqreturn_t img_ir_isr(int irq, void *dev_id)
+{
+	struct img_ir_priv *priv = dev_id;
+	u32 irq_status;
+
+	spin_lock(&priv->lock);
+	/* we have to clear irqs before reading */
+	irq_status = img_ir_read(priv, IMG_IR_IRQ_STATUS);
+	img_ir_write(priv, IMG_IR_IRQ_CLEAR, irq_status);
+
+	/* don't handle valid data irqs if we're only interested in matches */
+	irq_status &= img_ir_read(priv, IMG_IR_IRQ_ENABLE);
+
+	/* hand off edge interrupts to raw decode handler */
+	if (irq_status & IMG_IR_IRQ_EDGE && img_ir_raw_enabled(&priv->raw))
+		img_ir_isr_raw(priv, irq_status);
+
+	/* hand off hardware match interrupts to hardware decode handler */
+	if (irq_status & (IMG_IR_IRQ_DATA_MATCH |
+			  IMG_IR_IRQ_DATA_VALID |
+			  IMG_IR_IRQ_DATA2_VALID) &&
+	    img_ir_hw_enabled(&priv->hw))
+		img_ir_isr_hw(priv, irq_status);
+
+	spin_unlock(&priv->lock);
+	return IRQ_HANDLED;
+}
+
+static void img_ir_setup(struct img_ir_priv *priv)
+{
+	/* start off with interrupts disabled */
+	img_ir_write(priv, IMG_IR_IRQ_ENABLE, 0);
+
+	img_ir_setup_raw(priv);
+	img_ir_setup_hw(priv);
+
+	if (!IS_ERR(priv->clk))
+		clk_prepare_enable(priv->clk);
+}
+
+static void img_ir_ident(struct img_ir_priv *priv)
+{
+	u32 core_rev = img_ir_read(priv, IMG_IR_CORE_REV);
+
+	dev_info(priv->dev,
+		 "IMG IR Decoder (%d.%d.%d.%d) probed successfully\n",
+		 (core_rev & IMG_IR_DESIGNER) >> IMG_IR_DESIGNER_SHIFT,
+		 (core_rev & IMG_IR_MAJOR_REV) >> IMG_IR_MAJOR_REV_SHIFT,
+		 (core_rev & IMG_IR_MINOR_REV) >> IMG_IR_MINOR_REV_SHIFT,
+		 (core_rev & IMG_IR_MAINT_REV) >> IMG_IR_MAINT_REV_SHIFT);
+	dev_info(priv->dev, "Modes:%s%s\n",
+		 img_ir_hw_enabled(&priv->hw) ? " hardware" : "",
+		 img_ir_raw_enabled(&priv->raw) ? " raw" : "");
+}
+
+static int img_ir_probe(struct platform_device *pdev)
+{
+	struct img_ir_priv *priv;
+	struct resource *res_regs;
+	int irq, error, error2;
+
+	/* Get resources from platform device */
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "cannot find IRQ resource\n");
+		return irq;
+	}
+
+	/* Private driver data */
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&pdev->dev, "cannot allocate device data\n");
+		return -ENOMEM;
+	}
+	platform_set_drvdata(pdev, priv);
+	priv->dev = &pdev->dev;
+	spin_lock_init(&priv->lock);
+
+	/* Ioremap the registers */
+	res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->reg_base = devm_ioremap_resource(&pdev->dev, res_regs);
+	if (IS_ERR(priv->reg_base))
+		return PTR_ERR(priv->reg_base);
+
+	/* Get clock */
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk))
+		dev_warn(&pdev->dev, "cannot get clock resource\n");
+
+	/* Set up raw & hw decoder */
+	error = img_ir_probe_raw(priv);
+	error2 = img_ir_probe_hw(priv);
+	if (error && error2)
+		return (error == -ENODEV) ? error2 : error;
+
+	/* Get the IRQ */
+	priv->irq = irq;
+	error = request_irq(priv->irq, img_ir_isr, 0, "img-ir", priv);
+	if (error) {
+		dev_err(&pdev->dev, "cannot register IRQ %u\n",
+			priv->irq);
+		error = -EIO;
+		goto err_irq;
+	}
+
+	img_ir_ident(priv);
+	img_ir_setup(priv);
+
+	return 0;
+
+err_irq:
+	img_ir_remove_hw(priv);
+	img_ir_remove_raw(priv);
+	return error;
+}
+
+static int img_ir_remove(struct platform_device *pdev)
+{
+	struct img_ir_priv *priv = platform_get_drvdata(pdev);
+
+	free_irq(priv->irq, img_ir_isr);
+	img_ir_remove_hw(priv);
+	img_ir_remove_raw(priv);
+
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(img_ir_pmops, img_ir_suspend, img_ir_resume);
+
+static const struct of_device_id img_ir_match[] = {
+	{ .compatible = "img,ir" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, img_ir_match);
+
+static struct platform_driver img_ir_driver = {
+	.driver = {
+		.name = "img-ir",
+		.owner	= THIS_MODULE,
+		.of_match_table	= img_ir_match,
+		.pm = &img_ir_pmops,
+	},
+	.probe = img_ir_probe,
+	.remove = img_ir_remove,
+};
+
+module_platform_driver(img_ir_driver);
+
+MODULE_AUTHOR("Imagination Technologies Ltd.");
+MODULE_DESCRIPTION("ImgTec IR");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/rc/img-ir/img-ir.h b/drivers/media/rc/img-ir/img-ir.h
new file mode 100644
index 000000000000..950cf90573c8
--- /dev/null
+++ b/drivers/media/rc/img-ir/img-ir.h
@@ -0,0 +1,170 @@
+/*
+ * ImgTec IR Decoder found in PowerDown Controller.
+ *
+ * Copyright 2010-2013 Imagination Technologies Ltd.
+ */
+
+#ifndef _IMG_IR_H_
+#define _IMG_IR_H_
+
+#include <linux/spinlock.h>
+
+#include "img-ir-raw.h"
+#include "img-ir-hw.h"
+
+/* registers */
+
+/* relative to the start of the IR block of registers */
+#define IMG_IR_CONTROL		0x00
+#define IMG_IR_STATUS		0x04
+#define IMG_IR_DATA_LW		0x08
+#define IMG_IR_DATA_UP		0x0c
+#define IMG_IR_LEAD_SYMB_TIMING	0x10
+#define IMG_IR_S00_SYMB_TIMING	0x14
+#define IMG_IR_S01_SYMB_TIMING	0x18
+#define IMG_IR_S10_SYMB_TIMING	0x1c
+#define IMG_IR_S11_SYMB_TIMING	0x20
+#define IMG_IR_FREE_SYMB_TIMING	0x24
+#define IMG_IR_POW_MOD_PARAMS	0x28
+#define IMG_IR_POW_MOD_ENABLE	0x2c
+#define IMG_IR_IRQ_MSG_DATA_LW	0x30
+#define IMG_IR_IRQ_MSG_DATA_UP	0x34
+#define IMG_IR_IRQ_MSG_MASK_LW	0x38
+#define IMG_IR_IRQ_MSG_MASK_UP	0x3c
+#define IMG_IR_IRQ_ENABLE	0x40
+#define IMG_IR_IRQ_STATUS	0x44
+#define IMG_IR_IRQ_CLEAR	0x48
+#define IMG_IR_IRCORE_ID	0xf0
+#define IMG_IR_CORE_REV		0xf4
+#define IMG_IR_CORE_DES1	0xf8
+#define IMG_IR_CORE_DES2	0xfc
+
+
+/* field masks */
+
+/* IMG_IR_CONTROL */
+#define IMG_IR_DECODEN		0x40000000
+#define IMG_IR_CODETYPE		0x30000000
+#define IMG_IR_CODETYPE_SHIFT		28
+#define IMG_IR_HDRTOG		0x08000000
+#define IMG_IR_LDRDEC		0x04000000
+#define IMG_IR_DECODINPOL	0x02000000	/* active high */
+#define IMG_IR_BITORIEN		0x01000000	/* MSB first */
+#define IMG_IR_D1VALIDSEL	0x00008000
+#define IMG_IR_BITINV		0x00000040	/* don't invert */
+#define IMG_IR_DECODEND2	0x00000010
+#define IMG_IR_BITORIEND2	0x00000002	/* MSB first */
+#define IMG_IR_BITINVD2		0x00000001	/* don't invert */
+
+/* IMG_IR_STATUS */
+#define IMG_IR_RXDVALD2		0x00001000
+#define IMG_IR_IRRXD		0x00000400
+#define IMG_IR_TOGSTATE		0x00000200
+#define IMG_IR_RXDVAL		0x00000040
+#define IMG_IR_RXDLEN		0x0000003f
+#define IMG_IR_RXDLEN_SHIFT		0
+
+/* IMG_IR_LEAD_SYMB_TIMING, IMG_IR_Sxx_SYMB_TIMING */
+#define IMG_IR_PD_MAX		0xff000000
+#define IMG_IR_PD_MAX_SHIFT		24
+#define IMG_IR_PD_MIN		0x00ff0000
+#define IMG_IR_PD_MIN_SHIFT		16
+#define IMG_IR_W_MAX		0x0000ff00
+#define IMG_IR_W_MAX_SHIFT		8
+#define IMG_IR_W_MIN		0x000000ff
+#define IMG_IR_W_MIN_SHIFT		0
+
+/* IMG_IR_FREE_SYMB_TIMING */
+#define IMG_IR_MAXLEN		0x0007e000
+#define IMG_IR_MAXLEN_SHIFT		13
+#define IMG_IR_MINLEN		0x00001f00
+#define IMG_IR_MINLEN_SHIFT		8
+#define IMG_IR_FT_MIN		0x000000ff
+#define IMG_IR_FT_MIN_SHIFT		0
+
+/* IMG_IR_POW_MOD_PARAMS */
+#define IMG_IR_PERIOD_LEN	0x3f000000
+#define IMG_IR_PERIOD_LEN_SHIFT		24
+#define IMG_IR_PERIOD_DUTY	0x003f0000
+#define IMG_IR_PERIOD_DUTY_SHIFT	16
+#define IMG_IR_STABLE_STOP	0x00003f00
+#define IMG_IR_STABLE_STOP_SHIFT	8
+#define IMG_IR_STABLE_START	0x0000003f
+#define IMG_IR_STABLE_START_SHIFT	0
+
+/* IMG_IR_POW_MOD_ENABLE */
+#define IMG_IR_POWER_OUT_EN	0x00000002
+#define IMG_IR_POWER_MOD_EN	0x00000001
+
+/* IMG_IR_IRQ_ENABLE, IMG_IR_IRQ_STATUS, IMG_IR_IRQ_CLEAR */
+#define IMG_IR_IRQ_DEC2_ERR	0x00000080
+#define IMG_IR_IRQ_DEC_ERR	0x00000040
+#define IMG_IR_IRQ_ACT_LEVEL	0x00000020
+#define IMG_IR_IRQ_FALL_EDGE	0x00000010
+#define IMG_IR_IRQ_RISE_EDGE	0x00000008
+#define IMG_IR_IRQ_DATA_MATCH	0x00000004
+#define IMG_IR_IRQ_DATA2_VALID	0x00000002
+#define IMG_IR_IRQ_DATA_VALID	0x00000001
+#define IMG_IR_IRQ_ALL		0x000000ff
+#define IMG_IR_IRQ_EDGE		(IMG_IR_IRQ_FALL_EDGE | IMG_IR_IRQ_RISE_EDGE)
+
+/* IMG_IR_CORE_ID */
+#define IMG_IR_CORE_ID		0x00ff0000
+#define IMG_IR_CORE_ID_SHIFT		16
+#define IMG_IR_CORE_CONFIG	0x0000ffff
+#define IMG_IR_CORE_CONFIG_SHIFT	0
+
+/* IMG_IR_CORE_REV */
+#define IMG_IR_DESIGNER		0xff000000
+#define IMG_IR_DESIGNER_SHIFT		24
+#define IMG_IR_MAJOR_REV	0x00ff0000
+#define IMG_IR_MAJOR_REV_SHIFT		16
+#define IMG_IR_MINOR_REV	0x0000ff00
+#define IMG_IR_MINOR_REV_SHIFT		8
+#define IMG_IR_MAINT_REV	0x000000ff
+#define IMG_IR_MAINT_REV_SHIFT		0
+
+struct device;
+struct clk;
+
+/**
+ * struct img_ir_priv - Private driver data.
+ * @next:		Next IR device's private driver data (to form a linked
+ *			list).
+ * @dev:		Platform device.
+ * @irq:		IRQ number.
+ * @clk:		Input clock.
+ * @reg_base:		Iomem base address of IR register block.
+ * @lock:		Protects IR registers and variables in this struct.
+ * @raw:		Driver data for raw decoder.
+ * @hw:			Driver data for hardware decoder.
+ */
+struct img_ir_priv {
+	/* this priv sits in a global list protected by img_ir_decoders_lock */
+	struct img_ir_priv	*next;
+
+	struct device		*dev;
+	int			irq;
+	struct clk		*clk;
+	void __iomem		*reg_base;
+	spinlock_t		lock;
+
+	struct img_ir_priv_raw	raw;
+	struct img_ir_priv_hw	hw;
+};
+
+/* Hardware access */
+
+static inline void img_ir_write(struct img_ir_priv *priv,
+				unsigned int reg_offs, unsigned int data)
+{
+	iowrite32(data, priv->reg_base + reg_offs);
+}
+
+static inline unsigned int img_ir_read(struct img_ir_priv *priv,
+				       unsigned int reg_offs)
+{
+	return ioread32(priv->reg_base + reg_offs);
+}
+
+#endif /* _IMG_IR_H_ */
-- 
1.8.1.2

Re: [PATCH 01/11] dt: binding: add binding for ImgTec IR block

[Mauro Carvalho Chehab]

Em Fri, 13 Dec 2013 15:12:49 +0000
James Hogan <james.hogan@imgtec.com> escreveu:

> Add device tree binding for ImgTec Consumer Infrared block.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: linux-media@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Landley <rob@landley.net>
> Cc: linux-doc@vger.kernel.org

It looks ok for me, but we should wait for a DT maintainer ack for
this one.

Regards,
Mauro

> ---
>  Documentation/devicetree/bindings/media/img-ir.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/img-ir.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/img-ir.txt b/Documentation/devicetree/bindings/media/img-ir.txt
> new file mode 100644
> index 000000000000..6f623b094ea6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/img-ir.txt
> @@ -0,0 +1,20 @@
> +* ImgTec Infrared (IR) decoder
> +
> +Required properties:
> +- compatible:		Should be "img,ir"
> +- reg:			Physical base address of the controller and length of
> +			memory mapped region.
> +- interrupts:		The interrupt specifier to the cpu.
> +
> +Optional properties:
> +- clocks:		Clock specifier for base clock.
> +			Defaults to 32.768KHz if not specified.
> +
> +Example:
> +
> +	ir@02006200 {
> +		compatible = "img,ir";
> +		reg = <0x02006200 0x100>;
> +		interrupts = <29 4>;
> +		clocks = <&clk_32khz>;
> +	};


-- 

Cheers,
Mauro

Re: [PATCH 01/11] dt: binding: add binding for ImgTec IR block

[Tomasz Figa]

Hi James,

On Friday 13 of December 2013 15:12:49 James Hogan wrote:
> Add device tree binding for ImgTec Consumer Infrared block.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: linux-media@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Landley <rob@landley.net>
> Cc: linux-doc@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/media/img-ir.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/img-ir.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/img-ir.txt b/Documentation/devicetree/bindings/media/img-ir.txt
> new file mode 100644
> index 000000000000..6f623b094ea6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/img-ir.txt
> @@ -0,0 +1,20 @@
> +* ImgTec Infrared (IR) decoder
> +
> +Required properties:
> +- compatible:		Should be "img,ir"

This compatible string isn't really very specific. Is there some IP
revision string that could be added, to account for possible design
changes that may require binding change?

> +- reg:			Physical base address of the controller and length of
> +			memory mapped region.
> +- interrupts:		The interrupt specifier to the cpu.
> +
> +Optional properties:
> +- clocks:		Clock specifier for base clock.
> +			Defaults to 32.768KHz if not specified.

To make the binding less fragile and allow interoperability with non-DT
platforms it may be better to provide also clock-names property (so you
can use clk_get(); that's a Linux implementation detail, though, but to
make our lives easier IMHO they should be sometimes considered too).

Best regards,
Tomasz

Re: [PATCH 01/11] dt: binding: add binding for ImgTec IR block

[James Hogan]

On 22/12/13 12:48, Tomasz Figa wrote:
>> diff --git a/Documentation/devicetree/bindings/media/img-ir.txt b/Documentation/devicetree/bindings/media/img-ir.txt
>> new file mode 100644
>> index 000000000000..6f623b094ea6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/img-ir.txt
>> @@ -0,0 +1,20 @@
>> +* ImgTec Infrared (IR) decoder
>> +
>> +Required properties:
>> +- compatible:		Should be "img,ir"
> 
> This compatible string isn't really very specific. Is there some IP
> revision string that could be added, to account for possible design
> changes that may require binding change?

Yes, agreed. I'll try and find a more unambiguous name for the IP block.

>> +- reg:			Physical base address of the controller and length of
>> +			memory mapped region.
>> +- interrupts:		The interrupt specifier to the cpu.
>> +
>> +Optional properties:
>> +- clocks:		Clock specifier for base clock.
>> +			Defaults to 32.768KHz if not specified.
> 
> To make the binding less fragile and allow interoperability with non-DT
> platforms it may be better to provide also clock-names property (so you
> can use clk_get(); that's a Linux implementation detail, though, but to
> make our lives easier IMHO they should be sometimes considered too).

Good idea. Looking at the hardware manual it actually describes 3 clock
inputs, and although only one is needed by the driver it makes sense for
the DT binding to be able to describe them all. I'll probably go with
these clock-names values:
"core": Core clock (32.867kHz)
"sys": System side (fast) clock
"mod": Power modulation clock

Cheers
James