linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/5] IR support for A83T - sunxi-ir driver update
@ 2017-12-16  2:49 Philipp Rossak
  2017-12-16  2:49 ` [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree Philipp Rossak
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16  2:49 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, maxime.ripard, wens, linux, sean,
	p.zabel, andi.shyti
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

This patch series adds support for the sunxi A83T ir module and enhances the sunxi-ir driver.
Right now the base clock frequency for the ir driver is a hard coded define and is set to 8 MHz.
This works for the most common ir receivers. On the Sinovoip Bananapi M3 the ir receiver needs,
a 3 MHz base clock frequency to work without problems with this driver (like in the legacy kernel).

To fix this issue I reworked the driver that this value could be set over the devicetree.

About 37 devices would have a devicetree change if this patch series would be applied.
Therfore I would like to ask you to give me some feedback about the patch series, before I finialize it.


Thanks in advance!

Philipp


Philipp Rossak (5):
  [media] rc: update sunxi-ir driver to get base frequency from
    devicetree
  [media] dt: bindings: Update binding documentation for sunxi IR
    controller
  ARM: dts: sun8i: a83t: Add the ir pin for the A83T
  ARM: dts: sun8i: a83t: Add support for the ir interface
  ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller

 Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 ++++++++------
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts         |  7 +++++++
 arch/arm/boot/dts/sun8i-a83t.dtsi                    | 15 +++++++++++++++
 drivers/media/rc/sunxi-cir.c                         | 20 +++++++++++---------
 4 files changed, 41 insertions(+), 15 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
  2017-12-16  2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
@ 2017-12-16  2:49 ` Philipp Rossak
  2017-12-16  9:18   ` Mauro Carvalho Chehab
  2017-12-16  2:49 ` [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller Philipp Rossak
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16  2:49 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, maxime.ripard, wens, linux, sean,
	p.zabel, andi.shyti
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

This patch updates the sunxi-ir driver to set the ir base clock from
devicetree.

This is neccessary since there are different ir recievers on the
market, that operate with different frequencys. So this value needs to
be set depending on the attached receiver.

Signed-off-by: Philipp Rossak <embed3d@gmail.com>
---
 drivers/media/rc/sunxi-cir.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 97f367b446c4..55b53d6463e9 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -72,12 +72,6 @@
 /* CIR_REG register idle threshold */
 #define REG_CIR_ITHR(val)    (((val) << 8) & (GENMASK(15, 8)))
 
-/* Required frequency for IR0 or IR1 clock in CIR mode */
-#define SUNXI_IR_BASE_CLK     8000000
-/* Frequency after IR internal divider  */
-#define SUNXI_IR_CLK          (SUNXI_IR_BASE_CLK / 64)
-/* Sample period in ns */
-#define SUNXI_IR_SAMPLE       (1000000000ul / SUNXI_IR_CLK)
 /* Noise threshold in samples  */
 #define SUNXI_IR_RXNOISE      1
 /* Idle Threshold in samples */
@@ -122,7 +116,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
 			/* for each bit in fifo */
 			dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
 			rawir.pulse = (dt & 0x80) != 0;
-			rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
+			rawir.duration = ((dt & 0x7f) + 1) * ir->rc->rx_resolution;
 			ir_raw_event_store_with_filter(ir->rc, &rawir);
 		}
 	}
@@ -148,6 +142,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	struct device_node *dn = dev->of_node;
 	struct resource *res;
 	struct sunxi_ir *ir;
+	u32 b_clk_freq;
 
 	ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
 	if (!ir)
@@ -172,6 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 		return PTR_ERR(ir->clk);
 	}
 
+	/* Required frequency for IR0 or IR1 clock in CIR mode */
+	if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) {
+		dev_err(dev, "failed to get ir base clock frequency.\n");
+		return -ENODATA;
+	}
+
 	/* Reset (optional) */
 	ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
 	if (IS_ERR(ir->rst))
@@ -180,7 +181,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
+	ret = clk_set_rate(ir->clk, b_clk_freq);
 	if (ret) {
 		dev_err(dev, "set ir base clock failed!\n");
 		goto exit_reset_assert;
@@ -225,7 +226,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
 	ir->rc->dev.parent = dev;
 	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
-	ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
+	/* Frequency after IR internal divider with sample period in ns */
+	ir->rc->rx_resolution = (1000000000ul / (b_clk_freq / 64));
 	ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
 	ir->rc->driver_name = SUNXI_IR_DEV;
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller
  2017-12-16  2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
  2017-12-16  2:49 ` [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree Philipp Rossak
@ 2017-12-16  2:49 ` Philipp Rossak
  2017-12-16 18:36   ` Rob Herring
  2017-12-16  2:49 ` [RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T Philipp Rossak
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16  2:49 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, maxime.ripard, wens, linux, sean,
	p.zabel, andi.shyti
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

This patch updates documentation for Device-Tree bindings for sunxi IR
controller and adds the new requiered property for the base clock frequency.

Signed-off-by: Philipp Rossak <embed3d@gmail.com>
---
 Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt
index 91648c569b1e..5f4960c61245 100644
--- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
+++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
@@ -1,12 +1,13 @@
 Device-Tree bindings for SUNXI IR controller found in sunXi SoC family
 
 Required properties:
-- compatible	    : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
-- clocks	    : list of clock specifiers, corresponding to
-		      entries in clock-names property;
-- clock-names	    : should contain "apb" and "ir" entries;
-- interrupts	    : should contain IR IRQ number;
-- reg		    : should contain IO map address for IR.
+- compatible	      : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
+- clocks	      : list of clock specifiers, corresponding to
+		        entries in clock-names property;
+- clock-names	      : should contain "apb" and "ir" entries;
+- interrupts	      : should contain IR IRQ number;
+- reg		      : should contain IO map address for IR.
+- base-clk-frequency  : should contain the base clock frequency
 
 Optional properties:
 - linux,rc-map-name: see rc.txt file in the same directory.
@@ -21,5 +22,6 @@ ir0: ir@1c21800 {
 	resets = <&apb0_rst 1>;
 	interrupts = <0 5 1>;
 	reg = <0x01C21800 0x40>;
+	base-clk-frequency = <8000000>;
 	linux,rc-map-name = "rc-rc6-mce";
 };
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T
  2017-12-16  2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
  2017-12-16  2:49 ` [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree Philipp Rossak
  2017-12-16  2:49 ` [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller Philipp Rossak
@ 2017-12-16  2:49 ` Philipp Rossak
  2017-12-16  2:49 ` [RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface Philipp Rossak
  2017-12-16  2:49 ` [RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller Philipp Rossak
  4 siblings, 0 replies; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16  2:49 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, maxime.ripard, wens, linux, sean,
	p.zabel, andi.shyti
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

The CIR Pin of the A83T is located at PL12

Signed-off-by: Philipp Rossak <embed3d@gmail.com>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 19acae1b4089..5edb645b506f 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -488,6 +488,11 @@
 				drive-strength = <20>;
 				bias-pull-up;
 			};
+
+			ir_pins_a: ir@0 {
+				pins = "PL12";
+				function = "s_cir_rx";
+			};
 		};
 
 		r_rsb: rsb@1f03400 {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface
  2017-12-16  2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
                   ` (2 preceding siblings ...)
  2017-12-16  2:49 ` [RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T Philipp Rossak
@ 2017-12-16  2:49 ` Philipp Rossak
  2017-12-16  2:49 ` [RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller Philipp Rossak
  4 siblings, 0 replies; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16  2:49 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, maxime.ripard, wens, linux, sean,
	p.zabel, andi.shyti
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

The ir interface is like the H3 at 0x01f02000 located and is exactly
the same. This patch adds support for the ir interface on the A83T.

Signed-off-by: Philipp Rossak <embed3d@gmail.com>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 5edb645b506f..5dbf2f0891c1 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -470,6 +470,16 @@
 			#reset-cells = <1>;
 		};
 
+		ir: ir@01f02000 {
+			compatible = "allwinner,sun5i-a13-ir";
+			clocks = <&r_ccu CLK_APB0_IR>, <&r_ccu CLK_IR>;
+			clock-names = "apb", "ir";
+			resets = <&r_ccu RST_APB0_IR>;
+			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x01f02000 0x40>;
+			status = "disabled";
+		};
+
 		r_pio: pinctrl@1f02c00 {
 			compatible = "allwinner,sun8i-a83t-r-pinctrl";
 			reg = <0x01f02c00 0x400>;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller
  2017-12-16  2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
                   ` (3 preceding siblings ...)
  2017-12-16  2:49 ` [RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface Philipp Rossak
@ 2017-12-16  2:49 ` Philipp Rossak
  4 siblings, 0 replies; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16  2:49 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, maxime.ripard, wens, linux, sean,
	p.zabel, andi.shyti
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

The Bananapi M3 has an onboard IR receiver.
This enables the onboard IR receiver subnode.
Other than the other IR receivers this one needs a base clock frequency
of 3000000 Hz (3 MHz), to be able to work.

Signed-off-by: Philipp Rossak <embed3d@gmail.com>
---
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++++++
 arch/arm/boot/dts/sun8i-a83t.dtsi            | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index c606af3dbfed..2c92c501cd59 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -88,6 +88,13 @@
 	/* TODO GL830 USB-to-SATA bridge downstream w/ GPIO power controls */
 };
 
+&ir {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ir_pins_a>;
+	base-clk-frequency = <3000000>;
+	status = "okay";
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 5dbf2f0891c1..679ce3a66b4b 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -470,7 +470,7 @@
 			#reset-cells = <1>;
 		};
 
-		ir: ir@01f02000 {
+		ir: ir@1f02000 {
 			compatible = "allwinner,sun5i-a13-ir";
 			clocks = <&r_ccu CLK_APB0_IR>, <&r_ccu CLK_IR>;
 			clock-names = "apb", "ir";
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
  2017-12-16  2:49 ` [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree Philipp Rossak
@ 2017-12-16  9:18   ` Mauro Carvalho Chehab
  2017-12-16 17:30     ` Philipp Rossak
  0 siblings, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-16  9:18 UTC (permalink / raw)
  To: Philipp Rossak
  Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux, sean, p.zabel,
	andi.shyti, linux-media, devicetree, linux-arm-kernel,
	linux-kernel, linux-sunxi

Em Sat, 16 Dec 2017 03:49:10 +0100
Philipp Rossak <embed3d@gmail.com> escreveu:

Hi Phillip,

This is not a full review of this patchset. I just want to point you
that you should keep supporting existing DT files.

> This patch updates the sunxi-ir driver to set the ir base clock from
> devicetree.
> 
> This is neccessary since there are different ir recievers on the
> market, that operate with different frequencys. So this value needs to
> be set depending on the attached receiver.

Please don't break backward compatibility with old DT files. In this
specific case, it seems simple enough to be backward-compatible.

> 
> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> ---
>  drivers/media/rc/sunxi-cir.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 97f367b446c4..55b53d6463e9 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -72,12 +72,6 @@
>  /* CIR_REG register idle threshold */
>  #define REG_CIR_ITHR(val)    (((val) << 8) & (GENMASK(15, 8)))
>  
> -/* Required frequency for IR0 or IR1 clock in CIR mode */
> -#define SUNXI_IR_BASE_CLK     8000000
> -/* Frequency after IR internal divider  */
> -#define SUNXI_IR_CLK          (SUNXI_IR_BASE_CLK / 64)

Keep those to definitions...

> -/* Sample period in ns */
> -#define SUNXI_IR_SAMPLE       (1000000000ul / SUNXI_IR_CLK)
>  /* Noise threshold in samples  */
>  #define SUNXI_IR_RXNOISE      1
>  /* Idle Threshold in samples */
> @@ -122,7 +116,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>  			/* for each bit in fifo */
>  			dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
>  			rawir.pulse = (dt & 0x80) != 0;
> -			rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
> +			rawir.duration = ((dt & 0x7f) + 1) * ir->rc->rx_resolution;
>  			ir_raw_event_store_with_filter(ir->rc, &rawir);
>  		}
>  	}
> @@ -148,6 +142,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	struct device_node *dn = dev->of_node;
>  	struct resource *res;
>  	struct sunxi_ir *ir;
> +	u32 b_clk_freq;
>  
>  	ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
>  	if (!ir)
> @@ -172,6 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  		return PTR_ERR(ir->clk);
>  	}
>  
> +	/* Required frequency for IR0 or IR1 clock in CIR mode */
> +	if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) {
> +		dev_err(dev, "failed to get ir base clock frequency.\n");
> +		return -ENODATA;
> +	}
> +

And here, instead of returning an error, if the property can't be read, 
it means it is an older DT file. Just default to SUNXI_IR_BASE_CLK. 
This will make it backward-compatible with old DT files that don't have
such property.

Regards,
Mauro


>  	/* Reset (optional) */
>  	ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>  	if (IS_ERR(ir->rst))
> @@ -180,7 +181,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
> +	ret = clk_set_rate(ir->clk, b_clk_freq);
>  	if (ret) {
>  		dev_err(dev, "set ir base clock failed!\n");
>  		goto exit_reset_assert;
> @@ -225,7 +226,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
>  	ir->rc->dev.parent = dev;
>  	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> -	ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
> +	/* Frequency after IR internal divider with sample period in ns */
> +	ir->rc->rx_resolution = (1000000000ul / (b_clk_freq / 64));
>  	ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
>  	ir->rc->driver_name = SUNXI_IR_DEV;
>  

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
  2017-12-16  9:18   ` Mauro Carvalho Chehab
@ 2017-12-16 17:30     ` Philipp Rossak
  0 siblings, 0 replies; 9+ messages in thread
From: Philipp Rossak @ 2017-12-16 17:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux, sean, p.zabel,
	andi.shyti, linux-media, devicetree, linux-arm-kernel,
	linux-kernel, linux-sunxi

Hey Mauro,

Thanks for your fast feedback!

I will rework the driver like you suggested it.

Does somebody have any concerns about the Devicetree property 
(base-clk-frequency = < frequency >;)?


Regards,
Philipp

On 16.12.2017 10:18, Mauro Carvalho Chehab wrote:
> Em Sat, 16 Dec 2017 03:49:10 +0100
> Philipp Rossak <embed3d@gmail.com> escreveu:
> 
> Hi Phillip,
> 
> This is not a full review of this patchset. I just want to point you
> that you should keep supporting existing DT files.
> 
>> This patch updates the sunxi-ir driver to set the ir base clock from
>> devicetree.
>>
>> This is neccessary since there are different ir recievers on the
>> market, that operate with different frequencys. So this value needs to
>> be set depending on the attached receiver.
> 
> Please don't break backward compatibility with old DT files. In this
> specific case, it seems simple enough to be backward-compatible.
> 
>>
>> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
>> ---
>>   drivers/media/rc/sunxi-cir.c | 20 +++++++++++---------
>>   1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
>> index 97f367b446c4..55b53d6463e9 100644
>> --- a/drivers/media/rc/sunxi-cir.c
>> +++ b/drivers/media/rc/sunxi-cir.c
>> @@ -72,12 +72,6 @@
>>   /* CIR_REG register idle threshold */
>>   #define REG_CIR_ITHR(val)    (((val) << 8) & (GENMASK(15, 8)))
>>   
>> -/* Required frequency for IR0 or IR1 clock in CIR mode */
>> -#define SUNXI_IR_BASE_CLK     8000000
>> -/* Frequency after IR internal divider  */
>> -#define SUNXI_IR_CLK          (SUNXI_IR_BASE_CLK / 64)
> 
> Keep those to definitions...
> 
>> -/* Sample period in ns */
>> -#define SUNXI_IR_SAMPLE       (1000000000ul / SUNXI_IR_CLK)
>>   /* Noise threshold in samples  */
>>   #define SUNXI_IR_RXNOISE      1
>>   /* Idle Threshold in samples */
>> @@ -122,7 +116,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>>   			/* for each bit in fifo */
>>   			dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
>>   			rawir.pulse = (dt & 0x80) != 0;
>> -			rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
>> +			rawir.duration = ((dt & 0x7f) + 1) * ir->rc->rx_resolution;
>>   			ir_raw_event_store_with_filter(ir->rc, &rawir);
>>   		}
>>   	}
>> @@ -148,6 +142,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>>   	struct device_node *dn = dev->of_node;
>>   	struct resource *res;
>>   	struct sunxi_ir *ir;
>> +	u32 b_clk_freq;
>>   
>>   	ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
>>   	if (!ir)
>> @@ -172,6 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>>   		return PTR_ERR(ir->clk);
>>   	}
>>   
>> +	/* Required frequency for IR0 or IR1 clock in CIR mode */
>> +	if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) {
>> +		dev_err(dev, "failed to get ir base clock frequency.\n");
>> +		return -ENODATA;
>> +	}
>> +
> 
> And here, instead of returning an error, if the property can't be read,
> it means it is an older DT file. Just default to SUNXI_IR_BASE_CLK.
> This will make it backward-compatible with old DT files that don't have
> such property.
> 
> Regards,
> Mauro
> 
> 
>>   	/* Reset (optional) */
>>   	ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>>   	if (IS_ERR(ir->rst))
>> @@ -180,7 +181,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> -	ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
>> +	ret = clk_set_rate(ir->clk, b_clk_freq);
>>   	if (ret) {
>>   		dev_err(dev, "set ir base clock failed!\n");
>>   		goto exit_reset_assert;
>> @@ -225,7 +226,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>>   	ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
>>   	ir->rc->dev.parent = dev;
>>   	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>> -	ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
>> +	/* Frequency after IR internal divider with sample period in ns */
>> +	ir->rc->rx_resolution = (1000000000ul / (b_clk_freq / 64));
>>   	ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
>>   	ir->rc->driver_name = SUNXI_IR_DEV;
>>   
> 
> Thanks,
> Mauro
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller
  2017-12-16  2:49 ` [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller Philipp Rossak
@ 2017-12-16 18:36   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2017-12-16 18:36 UTC (permalink / raw)
  To: Philipp Rossak
  Cc: mchehab, mark.rutland, maxime.ripard, wens, linux, sean, p.zabel,
	andi.shyti, linux-media, devicetree, linux-arm-kernel,
	linux-kernel, linux-sunxi

gOn Sat, Dec 16, 2017 at 03:49:11AM +0100, Philipp Rossak wrote:
> This patch updates documentation for Device-Tree bindings for sunxi IR
> controller and adds the new requiered property for the base clock frequency.
> 
> Signed-off-by: Philipp Rossak <embed3d@gmail.com>
> ---
>  Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt
> index 91648c569b1e..5f4960c61245 100644
> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
> @@ -1,12 +1,13 @@
>  Device-Tree bindings for SUNXI IR controller found in sunXi SoC family
>  
>  Required properties:
> -- compatible	    : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
> -- clocks	    : list of clock specifiers, corresponding to
> -		      entries in clock-names property;
> -- clock-names	    : should contain "apb" and "ir" entries;
> -- interrupts	    : should contain IR IRQ number;
> -- reg		    : should contain IO map address for IR.
> +- compatible	      : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir"
> +- clocks	      : list of clock specifiers, corresponding to
> +		        entries in clock-names property;
> +- clock-names	      : should contain "apb" and "ir" entries;
> +- interrupts	      : should contain IR IRQ number;
> +- reg		      : should contain IO map address for IR.
> +- base-clk-frequency  : should contain the base clock frequency

Use clock-frequency or assigned-clocks.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-12-16 18:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-16  2:49 [RFC 0/5] IR support for A83T - sunxi-ir driver update Philipp Rossak
2017-12-16  2:49 ` [RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree Philipp Rossak
2017-12-16  9:18   ` Mauro Carvalho Chehab
2017-12-16 17:30     ` Philipp Rossak
2017-12-16  2:49 ` [RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller Philipp Rossak
2017-12-16 18:36   ` Rob Herring
2017-12-16  2:49 ` [RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T Philipp Rossak
2017-12-16  2:49 ` [RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface Philipp Rossak
2017-12-16  2:49 ` [RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller Philipp Rossak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).