[PATCH 0/4] media: rc: sunxi-cir: support the A523/H728/T527 IR receiver

[PATCH 0/4] media: rc: sunxi-cir: support the A523/H728/T527 IR receiver

[Justin Suess]

This series adds support for the CIR receiver found in the Allwinner
A523/T527/H728 family (sun55i). The only board in this family that I
am in possession of that has the IR receiver is the x96qpro+, so it is
just enabled for that board. The Avaota A1 may have it, but I don't have
hardware to test, so it's just enabled on the x96qpro+ for now, but
left in the a523 DTSI for future use.

The sun55i family carries a newer revision of the sunxi CIR IP that is
not backwards-compatible with the prior hardware. So a new pulse
capture mode field in the control register resets to 0 by default,
which captures nothing, and the sample clock divider became selectable
instead of the fixed module clock / 64.

Solve this by adding the two new registers, enabling the setting of the
pulse capture mode register and the selection of the clock speed.

The new compatible is therefore added standalone, without the
usual sun6i-a31-ir fallback (it won't work).

Tested on an X96Q Pro+ (H728) TV box with the Google TV remote that
came in the box (using NEC keymap). Was able to press buttons and
get readings from lirc / ir-keytable.

The series is based on tag v7.2-rc1 on mainline.

Justin Suess (4):
  media: dt-bindings: allwinner,sun4i-a10-ir: add A523 compatible
  media: rc: sunxi-cir: add support for the A523
  arm64: dts: allwinner: a523: add IR receiver node
  arm64: dts: allwinner: a523: enable IR receiver on the X96Q Pro+

 .../media/allwinner,sun4i-a10-ir.yaml         |  1 +
 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 19 +++++
 .../dts/allwinner/sun55i-h728-x96qpro+.dts    |  4 +
 drivers/media/rc/sunxi-cir.c                  | 76 +++++++++++++++----
 4 files changed, 87 insertions(+), 13 deletions(-)

-- 
2.54.0

[PATCH 1/4] media: dt-bindings: allwinner,sun4i-a10-ir: add A523 compatible

[Justin Suess]

The A523 (sun55i) contains a newer revision of the sunxi CIR receiver.
It is not backwards-compatible with the A31 programming model: the
control register has a pulse capture mode field (bits [7:6]) that
resets to a value which captures no pulses at all, and the sample
clock divider is now selectable via SPLCFG bits [1:0] instead of
being fixed at the old module clock / 64.

Since a kernel driving it as an A31 CIR receives nothing, add the
compatible as a standalone entry rather than under the
allwinner,sun6i-a31-ir fallback.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 .../devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml        | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
index 42dfe22ad5f1..06656058ce91 100644
--- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
+++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
@@ -19,6 +19,7 @@ properties:
       - const: allwinner,sun4i-a10-ir
       - const: allwinner,sun5i-a13-ir
       - const: allwinner,sun6i-a31-ir
+      - const: allwinner,sun55i-a523-ir
       - items:
           - enum:
               - allwinner,suniv-f1c100s-ir
-- 
2.54.0

Re: [PATCH 1/4] media: dt-bindings: allwinner,sun4i-a10-ir: add A523 compatible

[Krzysztof Kozlowski]

On Thu, Jul 02, 2026 at 05:47:47PM -0400, Justin Suess wrote:
> The A523 (sun55i) contains a newer revision of the sunxi CIR receiver.
> It is not backwards-compatible with the A31 programming model: the
> control register has a pulse capture mode field (bits [7:6]) that
> resets to a value which captures no pulses at all, and the sample
> clock divider is now selectable via SPLCFG bits [1:0] instead of
> being fixed at the old module clock / 64.
> 
> Since a kernel driving it as an A31 CIR receives nothing, add the
> compatible as a standalone entry rather than under the
> allwinner,sun6i-a31-ir fallback.
> 
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
>  .../devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml        | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
> index 42dfe22ad5f1..06656058ce91 100644
> --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
> @@ -19,6 +19,7 @@ properties:
>        - const: allwinner,sun4i-a10-ir
>        - const: allwinner,sun5i-a13-ir
>        - const: allwinner,sun6i-a31-ir
> +      - const: allwinner,sun55i-a523-ir

I assume you are adding it in expected order that Allwinner is
following, not just randomly at the end of any list.

Also, please change all above entries to one enum, can be the same
commit.


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH 1/4] media: dt-bindings: allwinner,sun4i-a10-ir: add A523 compatible

[Justin Suess]

On Fri, Jul 03, 2026 at 12:56:00PM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jul 02, 2026 at 05:47:47PM -0400, Justin Suess wrote:
> > The A523 (sun55i) contains a newer revision of the sunxi CIR receiver.
> > It is not backwards-compatible with the A31 programming model: the
> > control register has a pulse capture mode field (bits [7:6]) that
> > resets to a value which captures no pulses at all, and the sample
> > clock divider is now selectable via SPLCFG bits [1:0] instead of
> > being fixed at the old module clock / 64.
> > 
> > Since a kernel driving it as an A31 CIR receives nothing, add the
> > compatible as a standalone entry rather than under the
> > allwinner,sun6i-a31-ir fallback.
> > 
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > ---
> >  .../devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml        | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
> > index 42dfe22ad5f1..06656058ce91 100644
> > --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
> > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
> > @@ -19,6 +19,7 @@ properties:
> >        - const: allwinner,sun4i-a10-ir
> >        - const: allwinner,sun5i-a13-ir
> >        - const: allwinner,sun6i-a31-ir
> > +      - const: allwinner,sun55i-a523-ir
> 
> I assume you are adding it in expected order that Allwinner is
> following, not just randomly at the end of any list.
>
I did it in product generation order, is that what is expected?
I know that the a523 series succeeded a31, so that's what I put for
ordering.
> Also, please change all above entries to one enum, can be the same
> commit.
> 
No problem, I'll do that for v2.  
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> 
> Best regards,
> Krzysztof
>

Re: [PATCH 1/4] media: dt-bindings: allwinner,sun4i-a10-ir: add A523 compatible

[Krzysztof Kozlowski]

On 03/07/2026 21:50, Justin Suess wrote:
> On Fri, Jul 03, 2026 at 12:56:00PM +0200, Krzysztof Kozlowski wrote:
>> On Thu, Jul 02, 2026 at 05:47:47PM -0400, Justin Suess wrote:
>>> The A523 (sun55i) contains a newer revision of the sunxi CIR receiver.
>>> It is not backwards-compatible with the A31 programming model: the
>>> control register has a pulse capture mode field (bits [7:6]) that
>>> resets to a value which captures no pulses at all, and the sample
>>> clock divider is now selectable via SPLCFG bits [1:0] instead of
>>> being fixed at the old module clock / 64.
>>>
>>> Since a kernel driving it as an A31 CIR receives nothing, add the
>>> compatible as a standalone entry rather than under the
>>> allwinner,sun6i-a31-ir fallback.
>>>
>>> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
>>> ---
>>>  .../devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml        | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
>>> index 42dfe22ad5f1..06656058ce91 100644
>>> --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
>>> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-ir.yaml
>>> @@ -19,6 +19,7 @@ properties:
>>>        - const: allwinner,sun4i-a10-ir
>>>        - const: allwinner,sun5i-a13-ir
>>>        - const: allwinner,sun6i-a31-ir
>>> +      - const: allwinner,sun55i-a523-ir
>>
>> I assume you are adding it in expected order that Allwinner is
>> following, not just randomly at the end of any list.
>>
> I did it in product generation order, is that what is expected?

Don't ask me, I have no clue. There is only one recommended order for
every SoC and that is alphabetical. One or two platforms opted out, so
it's your task or maintainers job to remember the order - I just ask if
you keep that one here.


Best regards,
Krzysztof

[PATCH 2/4] media: rc: sunxi-cir: add support for the A523

[Justin Suess]

The A523 (sun55i) has a newer revision of the CIR receiver IP. Two
register fields that do not exist on older SoCs must be programmed
for reception to work:

 - CTL bits [7:6] select which pulse polarities are captured into the
   RX FIFO. The reset value of 0 captures nothing, so program "both
   pulse" mode, which captures regardless of header polarity.

 - SPLCFG (the sample configuration register) bits [1:0] select the
   sample clock as a division of the module clock, replacing the
   fixed module clock / 64 sample rate of the older IP. Select
   module clock / 256, which together with the 24 MHz module clock
   used on the A523 gives a 10.7 μs sample period, close to the 8 μs
   of the previous 8 MHz / 64 configuration, and keeps the default
   125 ms idle timeout representable in the 8-bit idle threshold
   field.

Parameterize the sample divisor in the resolution/timeout
calculations, which older SoCs keep at the fixed 64, and add the
A523 quirks and compatible.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 drivers/media/rc/sunxi-cir.c | 76 ++++++++++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 13 deletions(-)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index cb4c56bf0752..82ada9dc0347 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -31,6 +31,11 @@
 /* CIR mode */
 #define REG_CTL_MD			(BIT(4) | BIT(5))
 
+/* Pulse mode selector (bits [7:6]) */
+#define REG_CTL_PMD(m)			((m) << 6)
+/* Capture both pulse polarities */
+#define REG_CTL_PMD_BOTH		REG_CTL_PMD(1)
+
 /* Rx Config */
 #define SUNXI_IR_RXCTL_REG    0x10
 /* Pulse Polarity Invert flag */
@@ -66,6 +71,13 @@
 
 /* IR Sample Config */
 #define SUNXI_IR_CIR_REG      0x34
+/*
+ * Sample clock divider select (bits [1:0]), present on newer IP revisions
+ * (e.g. sun55i). Selects the sample clock as a fraction of the module clock;
+ * must be programmed for the sampler to run. Older SoCs lack the field and
+ * use a fixed module-clock/64 sample rate, so they leave it 0.
+ */
+#define REG_CIR_SDIV(val)    ((val) & GENMASK(1, 0))
 /* CIR_REG register noise threshold */
 #define REG_CIR_NTHR(val)    (((val) << 2) & (GENMASK(7, 2)))
 /* CIR_REG register idle threshold */
@@ -73,6 +85,8 @@
 
 /* Required frequency for IR0 or IR1 clock in CIR mode (default) */
 #define SUNXI_IR_BASE_CLK     8000000
+/* Default sample clock divisor: module clock / 64 (legacy fixed rate) */
+#define SUNXI_IR_SAMPLE_DIV   64
 /* Noise threshold in samples  */
 #define SUNXI_IR_RXNOISE      1
 
@@ -81,10 +95,18 @@
  *
  * @has_reset: SoC needs reset deasserted.
  * @fifo_size: size of the fifo.
+ * @both_pulse: program the CTRL pulse-mode field (newer IP revisions).
+ * @sample_div_sel: value for the SPLCFG sample-clock divider field (0 on
+ *		    legacy SoCs that lack the field).
+ * @sample_divisor: module-clock divisor that yields the sample clock; matches
+ *		    @sample_div_sel on newer IP, or the fixed /64 on legacy SoCs.
  */
 struct sunxi_ir_quirks {
 	bool		has_reset;
 	int		fifo_size;
+	bool		both_pulse;
+	u8		sample_div_sel;
+	u32		sample_divisor;
 };
 
 struct sunxi_ir {
@@ -92,6 +114,9 @@ struct sunxi_ir {
 	void __iomem    *base;
 	int             irq;
 	int		fifo_size;
+	bool		both_pulse;
+	u8		sample_div_sel;
+	u32		sample_divisor;
 	struct clk      *clk;
 	struct clk      *apb_clk;
 	struct reset_control *rst;
@@ -140,17 +165,19 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
 }
 
 /* Convert idle threshold to usec */
-static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int ithr)
+static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int div,
+				       unsigned int ithr)
 {
 	return DIV_ROUND_CLOSEST(USEC_PER_SEC * (ithr + 1),
-				 base_clk / (128 * 64));
+				 base_clk / (128 * div));
 }
 
 /* Convert usec to idle threshold */
-static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int usec)
+static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int div,
+				       unsigned int usec)
 {
 	/* make sure we don't end up with a timeout less than requested */
-	return DIV_ROUND_UP((base_clk / (128 * 64)) * usec,  USEC_PER_SEC) - 1;
+	return DIV_ROUND_UP((base_clk / (128 * div)) * usec,  USEC_PER_SEC) - 1;
 }
 
 static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
@@ -158,15 +185,17 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
 	struct sunxi_ir *ir = rc_dev->priv;
 	unsigned int base_clk = clk_get_rate(ir->clk);
 
-	unsigned int ithr = sunxi_usec_to_ithr(base_clk, timeout);
+	unsigned int ithr = sunxi_usec_to_ithr(base_clk, ir->sample_divisor,
+					       timeout);
 
 	dev_dbg(rc_dev->dev.parent, "setting idle threshold to %u\n", ithr);
 
-	/* Set noise threshold and idle threshold */
-	writel(REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
+	/* Set sample clock divider, noise threshold and idle threshold */
+	writel(REG_CIR_SDIV(ir->sample_div_sel) |
+	       REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
 	       ir->base + SUNXI_IR_CIR_REG);
 
-	rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ithr);
+	rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ir->sample_divisor, ithr);
 
 	return 0;
 }
@@ -193,8 +222,14 @@ static int sunxi_ir_hw_init(struct device *dev)
 		goto exit_disable_apb_clk;
 	}
 
-	/* Enable CIR Mode */
-	writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
+	/*
+	 * Enable CIR Mode. On newer IP revisions the pulse-mode field must
+	 * also be set, otherwise no pulses are captured into the RX FIFO.
+	 */
+	tmp = REG_CTL_MD;
+	if (ir->both_pulse)
+		tmp |= REG_CTL_PMD_BOTH;
+	writel(tmp, ir->base + SUNXI_IR_CTL_REG);
 
 	/* Set noise threshold and idle threshold */
 	sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
@@ -271,6 +306,9 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	}
 
 	ir->fifo_size = quirks->fifo_size;
+	ir->both_pulse = quirks->both_pulse;
+	ir->sample_div_sel = quirks->sample_div_sel;
+	ir->sample_divisor = quirks->sample_divisor ?: SUNXI_IR_SAMPLE_DIV;
 
 	/* Clock */
 	ir->apb_clk = devm_clk_get(dev, "apb");
@@ -325,10 +363,10 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	ir->rc->dev.parent = dev;
 	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
 	/* Frequency after IR internal divider with sample period in us */
-	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
+	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / ir->sample_divisor));
 	ir->rc->timeout = IR_DEFAULT_TIMEOUT;
-	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
-	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
+	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 0);
+	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 255);
 	ir->rc->s_timeout = sunxi_ir_set_timeout;
 	ir->rc->driver_name = SUNXI_IR_DEV;
 
@@ -395,6 +433,14 @@ static const struct sunxi_ir_quirks sun6i_a31_ir_quirks = {
 	.fifo_size = 64,
 };
 
+static const struct sunxi_ir_quirks sun55i_a523_ir_quirks = {
+	.has_reset = true,
+	.fifo_size = 64,
+	.both_pulse = true,
+	.sample_div_sel = 2,	/* sample clock = module clock / 256 */
+	.sample_divisor = 256,
+};
+
 static const struct of_device_id sunxi_ir_match[] = {
 	{
 		.compatible = "allwinner,sun4i-a10-ir",
@@ -408,6 +454,10 @@ static const struct of_device_id sunxi_ir_match[] = {
 		.compatible = "allwinner,sun6i-a31-ir",
 		.data = &sun6i_a31_ir_quirks,
 	},
+	{
+		.compatible = "allwinner,sun55i-a523-ir",
+		.data = &sun55i_a523_ir_quirks,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_ir_match);
-- 
2.54.0

Re: [PATCH 2/4] media: rc: sunxi-cir: add support for the A523

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect device registration order in probe and missing `rc_unregister_device` on failure leads to race conditions and Use-After-Free.
- [High] Rounding mismatch in timeout calculations causes the maximum timeout to wrap around to 0, breaking IR reception.
- [Medium] Malformed `clock-frequency` causes a division by zero kernel panic during timeout calculations.
- [Medium] Inconsistent use of DT-requested clock frequency versus actual clock rate introduces timing inaccuracies.
--

commit ca10429d8baf92c8b3413bded6e91ab88182edf2
Author: Justin Suess <utilityemal77@gmail.com>

media: rc: sunxi-cir: add support for the A523

This patch adds support for the A523 (sun55i) SoC to the sunxi-cir
driver. It introduces support for programming pulse polarities and a
parameterized sample clock divider required by the newer IP revision.

> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c

[ ... ]

> @@ -140,17 +165,19 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>  
>  /* Convert idle threshold to usec */
> -static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int ithr)
> +static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int div,
> +				       unsigned int ithr)
>  {
>  	return DIV_ROUND_CLOSEST(USEC_PER_SEC * (ithr + 1),
> -				 base_clk / (128 * 64));
> +				 base_clk / (128 * div));

[Severity: Medium]
This is a pre-existing issue, but does this calculation risk a division by
zero panic during device probe?

If the clock provider rate evaluates to a value smaller than 128 * div,
base_clk / (128 * div) evaluates to 0. This resulting zero is then passed
as the divisor to DIV_ROUND_CLOSEST(), which would cause a panic.

>  }
>  
>  /* Convert usec to idle threshold */
> -static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int usec)
> +static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int div,
> +				       unsigned int usec)
>  {
>  	/* make sure we don't end up with a timeout less than requested */
> -	return DIV_ROUND_UP((base_clk / (128 * 64)) * usec,  USEC_PER_SEC) - 1;
> +	return DIV_ROUND_UP((base_clk / (128 * div)) * usec,  USEC_PER_SEC) - 1;
>  }
>  
>  static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
>  {
>  	struct sunxi_ir *ir = rc_dev->priv;
>  	unsigned int base_clk = clk_get_rate(ir->clk);
>  
> -	unsigned int ithr = sunxi_usec_to_ithr(base_clk, timeout);
> +	unsigned int ithr = sunxi_usec_to_ithr(base_clk, ir->sample_divisor,
> +					       timeout);
>  
>  	dev_dbg(rc_dev->dev.parent, "setting idle threshold to %u\n", ithr);
>  
> -	/* Set noise threshold and idle threshold */
> -	writel(REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
> +	/* Set sample clock divider, noise threshold and idle threshold */
> +	writel(REG_CIR_SDIV(ir->sample_div_sel) |
> +	       REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
>  	       ir->base + SUNXI_IR_CIR_REG);

[Severity: High]
This is a pre-existing issue, but does the rounding mismatch between
sunxi_ithr_to_usec() and sunxi_usec_to_ithr() cause the maximum timeout
to wrap around to 0?

Because sunxi_ithr_to_usec() uses DIV_ROUND_CLOSEST, it can round the
fractional value up when calculating the maximum timeout. If userspace
requests this maximum timeout, sunxi_usec_to_ithr() uses DIV_ROUND_UP,
which pushes the resulting threshold over the maximum of 255 to 256.

Since REG_CIR_ITHR masks the value to 8 bits, passing 256 wraps it to 0
when written to the hardware, which would immediately break IR reception.

[ ... ]

> @@ -325,10 +363,10 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	ir->rc->dev.parent = dev;
>  	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>  	/* Frequency after IR internal divider with sample period in us */
> -	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
> +	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / ir->sample_divisor));

[Severity: Medium]
This is a pre-existing issue, but does this calculation introduce timing
inaccuracies by using the DT-requested clock frequency instead of the
actual clock rate?

If the clock provider rounds the requested rate set earlier in
sunxi_ir_probe(), using the raw b_clk_freq here instead of
clk_get_rate(ir->clk) means the software's sample period drifts from the
actual hardware timing.

[Severity: High]
This is a pre-existing issue, but does the registration order in
sunxi_ir_probe() create a race condition and potential Use-After-Free?

Looking later in sunxi_ir_probe(), rc_register_device() is called before
sunxi_ir_hw_init() enables the hardware clocks:

sunxi_ir_probe() {
    ...
    ret = rc_register_device(ir->rc);
    ...
    ret = sunxi_ir_hw_init(dev);
    ...
}

If userspace accesses the device via ioctl before the hardware clocks are
enabled, it will access clock-gated memory and cause a bus fault.

Additionally, if probe fails during hardware initialization or IRQ request,
it branches to the exit_free_dev label:

exit_free_dev:
    rc_free_device(ir->rc);

This only calls rc_free_device() without calling rc_unregister_device()
first. The device remains exposed to userspace while its backing driver
data is freed, which could lead to a Use-After-Free memory corruption.

>  	ir->rc->timeout = IR_DEFAULT_TIMEOUT;
> -	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
> -	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
> +	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 0);
> +	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 255);
>  	ir->rc->s_timeout = sunxi_ir_set_timeout;
>  	ir->rc->driver_name = SUNXI_IR_DEV;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702214750.3428694-1-utilityemal77@gmail.com?part=2

Re: [PATCH 2/4] media: rc: sunxi-cir: add support for the A523

[Andre Przywara]

On Thu,  2 Jul 2026 17:47:48 -0400
Justin Suess <utilityemal77@gmail.com> wrote:

Hi Justin,

many thanks for sending this!

> The A523 (sun55i) has a newer revision of the CIR receiver IP. Two
> register fields that do not exist on older SoCs must be programmed
> for reception to work:
> 
>  - CTL bits [7:6] select which pulse polarities are captured into the
>    RX FIFO. The reset value of 0 captures nothing, so program "both
>    pulse" mode, which captures regardless of header polarity.

Are you sure about that? The manual says that *both* the 0b00 (reset
default) and 0b01 values capture both edges, and actually the H6, A133
and H616 have the same bits, and it apparently works there.

I don't see those bits documented in the A64 (and earlier), but haven't 
checked yet whether they exist there regardless or have an effect.
So I think we should force those bits either to 0 or to 1, depending on 
how those bits behave on A64 and before, and how compatible this is with 
H6, A133, H616. I will try to run some experiments on the weekend.

>  - SPLCFG (the sample configuration register) bits [1:0] select the
>    sample clock as a division of the module clock, replacing the
>    fixed module clock / 64 sample rate of the older IP.

That's not fully correct: even the A20(!) has these two bits, actually
there is a third bit, held in bit 24 (because reasons). All those bits
reset to 0, which is encoded as /64, so this is where the rate comes
from. And sunxi_ir_probe() sets the IR clock to 8MHz, which should end
up as 24MHz / 3, on all chips, including the A523.

So what is going on here? Is the manual wrong, about those bits, or the 
clock sources?
Can you point to the BSP sources, if you used those?

>    module clock / 256, which together with the 24 MHz module clock

Why is the A523 mod clock set to 24 MHz? You seem to do this in the DT, 
overriding the 8MHz default? The driver clearly has a clk_set_rate() 
call with that default 8MHz as an argument, and I don't think we should 
deviate from that, unless there are good reasons. The sample clock 
should be more of a driver/subsystem decision, not a a device one.

>    used on the A523 gives a 10.7 μs sample period, close to the 8 μs
>    of the previous 8 MHz / 64 configuration, and keeps the default
>    125 ms idle timeout representable in the 8-bit idle threshold

This is some good info that helps people understand the reasoning behind 
those timing values. Please put this in a comment near the top of the file.
But actually: how does this compute? With an 8us sample clock period, 
the 8-bit ATHR field only covers 2 ms. And I don't see us setting the 
ATHC bit to bump this by 128.

Cheers,
Andre

>    field.
> 
> Parameterize the sample divisor in the resolution/timeout
> calculations, which older SoCs keep at the fixed 64, and add the
> A523 quirks and compatible.
> 
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
>  drivers/media/rc/sunxi-cir.c | 76 ++++++++++++++++++++++++++++++------
>  1 file changed, 63 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index cb4c56bf0752..82ada9dc0347 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -31,6 +31,11 @@
>  /* CIR mode */
>  #define REG_CTL_MD			(BIT(4) | BIT(5))
>  
> +/* Pulse mode selector (bits [7:6]) */
> +#define REG_CTL_PMD(m)			((m) << 6)
> +/* Capture both pulse polarities */
> +#define REG_CTL_PMD_BOTH		REG_CTL_PMD(1)
> +
>  /* Rx Config */
>  #define SUNXI_IR_RXCTL_REG    0x10
>  /* Pulse Polarity Invert flag */
> @@ -66,6 +71,13 @@
>  
>  /* IR Sample Config */
>  #define SUNXI_IR_CIR_REG      0x34
> +/*
> + * Sample clock divider select (bits [1:0]), present on newer IP revisions
> + * (e.g. sun55i). Selects the sample clock as a fraction of the module clock;
> + * must be programmed for the sampler to run. Older SoCs lack the field and
> + * use a fixed module-clock/64 sample rate, so they leave it 0.
> + */
> +#define REG_CIR_SDIV(val)    ((val) & GENMASK(1, 0))
>  /* CIR_REG register noise threshold */
>  #define REG_CIR_NTHR(val)    (((val) << 2) & (GENMASK(7, 2)))
>  /* CIR_REG register idle threshold */
> @@ -73,6 +85,8 @@
>  
>  /* Required frequency for IR0 or IR1 clock in CIR mode (default) */
>  #define SUNXI_IR_BASE_CLK     8000000
> +/* Default sample clock divisor: module clock / 64 (legacy fixed rate) */
> +#define SUNXI_IR_SAMPLE_DIV   64
>  /* Noise threshold in samples  */
>  #define SUNXI_IR_RXNOISE      1
>  
> @@ -81,10 +95,18 @@
>   *
>   * @has_reset: SoC needs reset deasserted.
>   * @fifo_size: size of the fifo.
> + * @both_pulse: program the CTRL pulse-mode field (newer IP revisions).

As mentioned above, those bits exist in earlier IP as well. Typically 
non-implemented bits in Allwinner IP as RES0, so I think we can program 
them unconditionally (and should on H6/A133/H616) and don't need a 
quirks flag.

> + * @sample_div_sel: value for the SPLCFG sample-clock divider field (0 on
> + *		    legacy SoCs that lack the field).

Same here: those bits exist back to the A20, even. And their meaning 
didn't change, if I see this correctly. So no quirk needed, instead we 
should program them explicitly to the value we want (probably 0).

> + * @sample_divisor: module-clock divisor that yields the sample clock; matches
> + *		    @sample_div_sel on newer IP, or the fixed /64 on legacy SoCs.

That looks odd: why do we have that value in the first place? Following 
the things I mention above, the divisor shouldn't be different on the 
A523. And also, I think we should just do the math in the driver, and 
calculate the divisor, based on some timing requirement. Which could be 
something like: aim for a clock period of 8us. Though all the parameters 
seem to be stable: the 24 MHz OSC input, the dividers in the mod clock, 
and the post dividers in register 0x34. So there wouldn't be much of a 
calculation, really. But I still think the driver can figure this out 
itself, and doesn't need explicit telling of a divisor.

So I think we would need a separate patch to fix up driver operation 
before A523. Then the A523 bits should go on top of this. And maybe make 
this two patches, one for the edge sample bits, one for the clock 
calculation.

Cheers,
Andre

>   */
>  struct sunxi_ir_quirks {
>  	bool		has_reset;
>  	int		fifo_size;
> +	bool		both_pulse;
> +	u8		sample_div_sel;
> +	u32		sample_divisor;
>  };
>  
>  struct sunxi_ir {
> @@ -92,6 +114,9 @@ struct sunxi_ir {
>  	void __iomem    *base;
>  	int             irq;
>  	int		fifo_size;
> +	bool		both_pulse;
> +	u8		sample_div_sel;
> +	u32		sample_divisor;
>  	struct clk      *clk;
>  	struct clk      *apb_clk;
>  	struct reset_control *rst;
> @@ -140,17 +165,19 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>  }
>  
>  /* Convert idle threshold to usec */
> -static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int ithr)
> +static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int div,
> +				       unsigned int ithr)
>  {
>  	return DIV_ROUND_CLOSEST(USEC_PER_SEC * (ithr + 1),
> -				 base_clk / (128 * 64));
> +				 base_clk / (128 * div));
>  }
>  
>  /* Convert usec to idle threshold */
> -static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int usec)
> +static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int div,
> +				       unsigned int usec)
>  {
>  	/* make sure we don't end up with a timeout less than requested */
> -	return DIV_ROUND_UP((base_clk / (128 * 64)) * usec,  USEC_PER_SEC) - 1;
> +	return DIV_ROUND_UP((base_clk / (128 * div)) * usec,  USEC_PER_SEC) - 1;
>  }
>  
>  static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
> @@ -158,15 +185,17 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
>  	struct sunxi_ir *ir = rc_dev->priv;
>  	unsigned int base_clk = clk_get_rate(ir->clk);
>  
> -	unsigned int ithr = sunxi_usec_to_ithr(base_clk, timeout);
> +	unsigned int ithr = sunxi_usec_to_ithr(base_clk, ir->sample_divisor,
> +					       timeout);
>  
>  	dev_dbg(rc_dev->dev.parent, "setting idle threshold to %u\n", ithr);
>  
> -	/* Set noise threshold and idle threshold */
> -	writel(REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
> +	/* Set sample clock divider, noise threshold and idle threshold */
> +	writel(REG_CIR_SDIV(ir->sample_div_sel) |
> +	       REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
>  	       ir->base + SUNXI_IR_CIR_REG);
>  
> -	rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ithr);
> +	rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ir->sample_divisor, ithr);
>  
>  	return 0;
>  }
> @@ -193,8 +222,14 @@ static int sunxi_ir_hw_init(struct device *dev)
>  		goto exit_disable_apb_clk;
>  	}
>  
> -	/* Enable CIR Mode */
> -	writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
> +	/*
> +	 * Enable CIR Mode. On newer IP revisions the pulse-mode field must
> +	 * also be set, otherwise no pulses are captured into the RX FIFO.
> +	 */
> +	tmp = REG_CTL_MD;
> +	if (ir->both_pulse)
> +		tmp |= REG_CTL_PMD_BOTH;
> +	writel(tmp, ir->base + SUNXI_IR_CTL_REG);
>  
>  	/* Set noise threshold and idle threshold */
>  	sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
> @@ -271,6 +306,9 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	}
>  
>  	ir->fifo_size = quirks->fifo_size;
> +	ir->both_pulse = quirks->both_pulse;
> +	ir->sample_div_sel = quirks->sample_div_sel;
> +	ir->sample_divisor = quirks->sample_divisor ?: SUNXI_IR_SAMPLE_DIV;
>  
>  	/* Clock */
>  	ir->apb_clk = devm_clk_get(dev, "apb");
> @@ -325,10 +363,10 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  	ir->rc->dev.parent = dev;
>  	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>  	/* Frequency after IR internal divider with sample period in us */
> -	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
> +	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / ir->sample_divisor));
>  	ir->rc->timeout = IR_DEFAULT_TIMEOUT;
> -	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
> -	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
> +	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 0);
> +	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 255);
>  	ir->rc->s_timeout = sunxi_ir_set_timeout;
>  	ir->rc->driver_name = SUNXI_IR_DEV;
>  
> @@ -395,6 +433,14 @@ static const struct sunxi_ir_quirks sun6i_a31_ir_quirks = {
>  	.fifo_size = 64,
>  };
>  
> +static const struct sunxi_ir_quirks sun55i_a523_ir_quirks = {
> +	.has_reset = true,
> +	.fifo_size = 64,
> +	.both_pulse = true,
> +	.sample_div_sel = 2,	/* sample clock = module clock / 256 */
> +	.sample_divisor = 256,
> +};
> +
>  static const struct of_device_id sunxi_ir_match[] = {
>  	{
>  		.compatible = "allwinner,sun4i-a10-ir",
> @@ -408,6 +454,10 @@ static const struct of_device_id sunxi_ir_match[] = {
>  		.compatible = "allwinner,sun6i-a31-ir",
>  		.data = &sun6i_a31_ir_quirks,
>  	},
> +	{
> +		.compatible = "allwinner,sun55i-a523-ir",
> +		.data = &sun55i_a523_ir_quirks,
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, sunxi_ir_match);

Re: [PATCH 2/4] media: rc: sunxi-cir: add support for the A523

[Justin Suess]

On Fri, Jul 03, 2026 at 11:11:59AM +0200, Andre Przywara wrote:
> On Thu,  2 Jul 2026 17:47:48 -0400
> Justin Suess <utilityemal77@gmail.com> wrote:
> 
> Hi Justin,
> 
> many thanks for sending this!
> 
> > The A523 (sun55i) has a newer revision of the CIR receiver IP. Two
> > register fields that do not exist on older SoCs must be programmed
> > for reception to work:
> > 
> >  - CTL bits [7:6] select which pulse polarities are captured into the
> >    RX FIFO. The reset value of 0 captures nothing, so program "both
> >    pulse" mode, which captures regardless of header polarity.
> 
> Are you sure about that? The manual says that *both* the 0b00 (reset
> default) and 0b01 values capture both edges, and actually the H6, A133
> and H616 have the same bits, and it apparently works there.
>
No, actually. I literally just live dumped the registers on the vendor
kernel, (my android tv came pre-rooted for some reason) and interpreted
the purpose from the BSP source.

I am actually unaware there was such a manual. If you could point me to
it that'd be great.
> I don't see those bits documented in the A64 (and earlier), but haven't
> checked yet whether they exist there regardless or have an effect.
> So I think we should force those bits either to 0 or to 1, depending on how
> those bits behave on A64 and before, and how compatible this is with H6,
> A133, H616. I will try to run some experiments on the weekend.
>
Appreciate it!

I'll wait for the results of that experiment, I don't feel confident
enough that I wouldn't break things by even forcing the register to zero
without hardware to test on. So I preserved the existing behavior for the non
X98qpro+ board to be safe.

> >  - SPLCFG (the sample configuration register) bits [1:0] select the
> >    sample clock as a division of the module clock, replacing the
> >    fixed module clock / 64 sample rate of the older IP.
> 
> That's not fully correct: even the A20(!) has these two bits, actually
> there is a third bit, held in bit 24 (because reasons). All those bits
> reset to 0, which is encoded as /64, so this is where the rate comes
> from. And sunxi_ir_probe() sets the IR clock to 8MHz, which should end
> up as 24MHz / 3, on all chips, including the A523.
> 
> So what is going on here? Is the manual wrong, about those bits, or the
> clock sources?
> Can you point to the BSP sources, if you used those?
>

Actually I relied on a script running sunxi-dump on the vendor
kernel and dumping the registers for an initial implementation.

It didn't work, and I was getting stuck. (probably my sampling script
didn't dump the registers fast enough and didn't include all of the ones
needed).

Then I found https://github.com/chainsx/linux-sun55iw3-vendor

And saw this

  #define IR_SAMPLE_DEV		(0x2 << 0)	/* 24MHz/256 =93750Hz (~10.7us)*/

for the clock rate.

And then this one:

  #define IR_BOTH_PULSE		(0x1 << 6)

As and saw it being used as the pulse polarity.

(above are in bsp/drivers/ir-rx/sunxi-ir-rx.c)

And figured to give this one a try because everything I tried before
messing with the existing constants in mainline failed, and this was
the primary construct that didn't match up with mainline. 

Those two things were enough to get it working perfectly on my hardware.

So this is more based on "this is what worked on the hardware" + a
cursory read of the vendor source than based on any hardware technical
document or manual... I should have been more upfront about that.

So please point me to this manual and I can revise and figure out WHY
this works and improve this patch series. 

> >    module clock / 256, which together with the 24 MHz module clock
> 
> Why is the A523 mod clock set to 24 MHz? You seem to do this in the DT,
> overriding the 8MHz default? The driver clearly has a clk_set_rate() call
> with that default 8MHz as an argument, and I don't think we should deviate
> from that, unless there are good reasons. The sample clock should be more of
> a driver/subsystem decision, not a a device one.
> 

See above. This is just what worked on the hardware, and what the vendor
set. And it worked so I sent it. I didn't have access to the manual...

but agreed this needs justification and not just a handwaving "works on
my machine".

> >    used on the A523 gives a 10.7 μs sample period, close to the 8 μs
> >    of the previous 8 MHz / 64 configuration, and keeps the default
> >    125 ms idle timeout representable in the 8-bit idle threshold
> 
> This is some good info that helps people understand the reasoning behind
> those timing values. Please put this in a comment near the top of the file.
> But actually: how does this compute? With an 8us sample clock period, the
> 8-bit ATHR field only covers 2 ms. And I don't see us setting the ATHC bit
> to bump this by 128.
> 
I'll include it and anything else I can glean from your testing, and if
you can point me to the manual.

Thanks for the swift review, sorry should have made more obvious up
front this was a "tweaking until it works" implementation, not a ground
up spec based implementation.

> Cheers,
> Andre
> 
> >    field.
> > 
> > Parameterize the sample divisor in the resolution/timeout
> > calculations, which older SoCs keep at the fixed 64, and add the
> > A523 quirks and compatible.
> > 
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > ---
> >  drivers/media/rc/sunxi-cir.c | 76 ++++++++++++++++++++++++++++++------
> >  1 file changed, 63 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> > index cb4c56bf0752..82ada9dc0347 100644
> > --- a/drivers/media/rc/sunxi-cir.c
> > +++ b/drivers/media/rc/sunxi-cir.c
> > @@ -31,6 +31,11 @@
> >  /* CIR mode */
> >  #define REG_CTL_MD			(BIT(4) | BIT(5))
> > +/* Pulse mode selector (bits [7:6]) */
> > +#define REG_CTL_PMD(m)			((m) << 6)
> > +/* Capture both pulse polarities */
> > +#define REG_CTL_PMD_BOTH		REG_CTL_PMD(1)
> > +
> >  /* Rx Config */
> >  #define SUNXI_IR_RXCTL_REG    0x10
> >  /* Pulse Polarity Invert flag */
> > @@ -66,6 +71,13 @@
> >  /* IR Sample Config */
> >  #define SUNXI_IR_CIR_REG      0x34
> > +/*
> > + * Sample clock divider select (bits [1:0]), present on newer IP revisions
> > + * (e.g. sun55i). Selects the sample clock as a fraction of the module clock;
> > + * must be programmed for the sampler to run. Older SoCs lack the field and
> > + * use a fixed module-clock/64 sample rate, so they leave it 0.
> > + */
> > +#define REG_CIR_SDIV(val)    ((val) & GENMASK(1, 0))
> >  /* CIR_REG register noise threshold */
> >  #define REG_CIR_NTHR(val)    (((val) << 2) & (GENMASK(7, 2)))
> >  /* CIR_REG register idle threshold */
> > @@ -73,6 +85,8 @@
> >  /* Required frequency for IR0 or IR1 clock in CIR mode (default) */
> >  #define SUNXI_IR_BASE_CLK     8000000
> > +/* Default sample clock divisor: module clock / 64 (legacy fixed rate) */
> > +#define SUNXI_IR_SAMPLE_DIV   64
> >  /* Noise threshold in samples  */
> >  #define SUNXI_IR_RXNOISE      1
> > @@ -81,10 +95,18 @@
> >   *
> >   * @has_reset: SoC needs reset deasserted.
> >   * @fifo_size: size of the fifo.
> > + * @both_pulse: program the CTRL pulse-mode field (newer IP revisions).
> 
> As mentioned above, those bits exist in earlier IP as well. Typically
> non-implemented bits in Allwinner IP as RES0, so I think we can program them
> unconditionally (and should on H6/A133/H616) and don't need a quirks flag.
> 
Yeah, probably just setting them to zero is *fine* for the non-a523
hardware, but I didn't want to send patches for hardware I can't test.

Last thing I want to do is make someones TV remote stop working.

If your experiments reveal forcing zero is OK, than I'll default to
that.

> > + * @sample_div_sel: value for the SPLCFG sample-clock divider field (0 on
> > + *		    legacy SoCs that lack the field).
> 
> Same here: those bits exist back to the A20, even. And their meaning didn't
> change, if I see this correctly. So no quirk needed, instead we should
> program them explicitly to the value we want (probably 0).
> 
Sounds good
> > + * @sample_divisor: module-clock divisor that yields the sample clock; matches
> > + *		    @sample_div_sel on newer IP, or the fixed /64 on legacy SoCs.
> 
> That looks odd: why do we have that value in the first place? Following the
> things I mention above, the divisor shouldn't be different on the A523. And
> also, I think we should just do the math in the driver, and calculate the
> divisor, based on some timing requirement. Which could be something like:
> aim for a clock period of 8us. Though all the parameters seem to be stable:
> the 24 MHz OSC input, the dividers in the mod clock, and the post dividers
> in register 0x34. So there wouldn't be much of a calculation, really. But I
> still think the driver can figure this out itself, and doesn't need explicit
> telling of a divisor.
> 
This is just what worked for me; and what the vendor did. But you're
right, we shouldn't trust it if the quality of the bsp code is anything
to go off of.

> So I think we would need a separate patch to fix up driver operation before
> A523. Then the A523 bits should go on top of this. And maybe make this two
> patches, one for the edge sample bits, one for the clock calculation.
> 
Easy enough. I'll wait on your experiments, please send whatever you
found here while I prepare the seperate patches, and I'll adjust before
resending (no rush on the results, all I'd need is you forcing the
register to zero and verifying it works)

(and please if you could drop me this manual, would have saved me a lot of time!
I couldn't find it anywhere!)

Justin
> Cheers,
> Andre
> 
> >   */
> >  struct sunxi_ir_quirks {
> >  	bool		has_reset;
> >  	int		fifo_size;
> > +	bool		both_pulse;
> > +	u8		sample_div_sel;
> > +	u32		sample_divisor;
> >  };
> >  struct sunxi_ir {
> > @@ -92,6 +114,9 @@ struct sunxi_ir {
> >  	void __iomem    *base;
> >  	int             irq;
> >  	int		fifo_size;
> > +	bool		both_pulse;
> > +	u8		sample_div_sel;
> > +	u32		sample_divisor;
> >  	struct clk      *clk;
> >  	struct clk      *apb_clk;
> >  	struct reset_control *rst;
> > @@ -140,17 +165,19 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
> >  }
> >  /* Convert idle threshold to usec */
> > -static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int ithr)
> > +static unsigned int sunxi_ithr_to_usec(unsigned int base_clk, unsigned int div,
> > +				       unsigned int ithr)
> >  {
> >  	return DIV_ROUND_CLOSEST(USEC_PER_SEC * (ithr + 1),
> > -				 base_clk / (128 * 64));
> > +				 base_clk / (128 * div));
> >  }
> >  /* Convert usec to idle threshold */
> > -static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int usec)
> > +static unsigned int sunxi_usec_to_ithr(unsigned int base_clk, unsigned int div,
> > +				       unsigned int usec)
> >  {
> >  	/* make sure we don't end up with a timeout less than requested */
> > -	return DIV_ROUND_UP((base_clk / (128 * 64)) * usec,  USEC_PER_SEC) - 1;
> > +	return DIV_ROUND_UP((base_clk / (128 * div)) * usec,  USEC_PER_SEC) - 1;
> >  }
> >  static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
> > @@ -158,15 +185,17 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
> >  	struct sunxi_ir *ir = rc_dev->priv;
> >  	unsigned int base_clk = clk_get_rate(ir->clk);
> > -	unsigned int ithr = sunxi_usec_to_ithr(base_clk, timeout);
> > +	unsigned int ithr = sunxi_usec_to_ithr(base_clk, ir->sample_divisor,
> > +					       timeout);
> >  	dev_dbg(rc_dev->dev.parent, "setting idle threshold to %u\n", ithr);
> > -	/* Set noise threshold and idle threshold */
> > -	writel(REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
> > +	/* Set sample clock divider, noise threshold and idle threshold */
> > +	writel(REG_CIR_SDIV(ir->sample_div_sel) |
> > +	       REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
> >  	       ir->base + SUNXI_IR_CIR_REG);
> > -	rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ithr);
> > +	rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ir->sample_divisor, ithr);
> >  	return 0;
> >  }
> > @@ -193,8 +222,14 @@ static int sunxi_ir_hw_init(struct device *dev)
> >  		goto exit_disable_apb_clk;
> >  	}
> > -	/* Enable CIR Mode */
> > -	writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
> > +	/*
> > +	 * Enable CIR Mode. On newer IP revisions the pulse-mode field must
> > +	 * also be set, otherwise no pulses are captured into the RX FIFO.
> > +	 */
> > +	tmp = REG_CTL_MD;
> > +	if (ir->both_pulse)
> > +		tmp |= REG_CTL_PMD_BOTH;
> > +	writel(tmp, ir->base + SUNXI_IR_CTL_REG);
> >  	/* Set noise threshold and idle threshold */
> >  	sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
> > @@ -271,6 +306,9 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> >  	}
> >  	ir->fifo_size = quirks->fifo_size;
> > +	ir->both_pulse = quirks->both_pulse;
> > +	ir->sample_div_sel = quirks->sample_div_sel;
> > +	ir->sample_divisor = quirks->sample_divisor ?: SUNXI_IR_SAMPLE_DIV;
> >  	/* Clock */
> >  	ir->apb_clk = devm_clk_get(dev, "apb");
> > @@ -325,10 +363,10 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> >  	ir->rc->dev.parent = dev;
> >  	ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> >  	/* Frequency after IR internal divider with sample period in us */
> > -	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
> > +	ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / ir->sample_divisor));
> >  	ir->rc->timeout = IR_DEFAULT_TIMEOUT;
> > -	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
> > -	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
> > +	ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 0);
> > +	ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, ir->sample_divisor, 255);
> >  	ir->rc->s_timeout = sunxi_ir_set_timeout;
> >  	ir->rc->driver_name = SUNXI_IR_DEV;
> > @@ -395,6 +433,14 @@ static const struct sunxi_ir_quirks sun6i_a31_ir_quirks = {
> >  	.fifo_size = 64,
> >  };
> > +static const struct sunxi_ir_quirks sun55i_a523_ir_quirks = {
> > +	.has_reset = true,
> > +	.fifo_size = 64,
> > +	.both_pulse = true,
> > +	.sample_div_sel = 2,	/* sample clock = module clock / 256 */
> > +	.sample_divisor = 256,
> > +};
> > +
> >  static const struct of_device_id sunxi_ir_match[] = {
> >  	{
> >  		.compatible = "allwinner,sun4i-a10-ir",
> > @@ -408,6 +454,10 @@ static const struct of_device_id sunxi_ir_match[] = {
> >  		.compatible = "allwinner,sun6i-a31-ir",
> >  		.data = &sun6i_a31_ir_quirks,
> >  	},
> > +	{
> > +		.compatible = "allwinner,sun55i-a523-ir",
> > +		.data = &sun55i_a523_ir_quirks,
> > +	},
> >  	{}
> >  };
> >  MODULE_DEVICE_TABLE(of, sunxi_ir_match);
> 

[PATCH 3/4] arm64: dts: allwinner: a523: add IR receiver node

[Justin Suess]

The A523 has a CIR receiver in the RTC power domain, clocked from the
R-CCU, with its RX signal available on PL11.

Clock the module directly from the 24 MHz host oscillator; the driver
selects a /256 sample divider on this SoC, giving a sample period
close to the legacy 8 MHz / 64 configuration of older SoCs.

Keep the node disabled by default; boards with an IR receiver can
enable it.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index ca6a16807049..5e46c4b1ee61 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -927,6 +927,25 @@ r_i2c_pins: r-i2c-pins {
 				allwinner,pinmux = <2>;
 				function = "r_i2c0";
 			};
+
+			r_ir_rx_pin: r-ir-rx-pin {
+				pins = "PL11";
+				allwinner,pinmux = <2>;
+				function = "s_cir";
+			};
+		};
+
+		r_ir: ir@7040000 {
+			compatible = "allwinner,sun55i-a523-ir";
+			reg = <0x07040000 0x400>;
+			interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&r_ccu CLK_BUS_R_IR_RX>, <&r_ccu CLK_R_IR_RX>;
+			clock-names = "apb", "ir";
+			clock-frequency = <24000000>;
+			resets = <&r_ccu RST_BUS_R_IR_RX>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&r_ir_rx_pin>;
+			status = "disabled";
 		};
 
 		pck600: power-controller@7060000 {
-- 
2.54.0

[PATCH 4/4] arm64: dts: allwinner: a523: enable IR receiver on the X96Q Pro+

[Justin Suess]

The X96Q Pro+ TV box has an IR receiver window on the front panel,
wired to the SoC CIR input on PL11.

Enable the CIR receiver.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts b/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
index a96927fbdadd..de2bdc21c781 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts
@@ -266,6 +266,10 @@ reg_dcdc3_323: dcdc3 {
 	};
 };
 
+&r_ir {
+	status = "okay";
+};
+
 &r_pio {
 /*
  * Specifying the supply would create a circular dependency.
-- 
2.54.0