diff mbox

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

Message ID 1386947579-26703-2-git-send-email-james.hogan@imgtec.com
State Superseded, archived
Headers show

Commit Message

James Hogan Dec. 13, 2013, 3:12 p.m. UTC
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

Comments

Mauro Carvalho Chehab Dec. 22, 2013, 10:56 a.m. UTC | #1
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>;
> +	};
Tomasz Figa Dec. 22, 2013, 12:48 p.m. UTC | #2
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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Hogan Dec. 23, 2013, 10:41 a.m. UTC | #3
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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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>;
+	};