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

From: Robin Murphy
Date: Mon Jul 19 2021 - 06:41:03 EST

Hi Martin,

(just reading this patch out of passing curiosity...)

On 2021-07-17 20:57, Martin Blumenstingl wrote:
Hi Viktor,

On Fri, Jul 16, 2021 at 4:45 PM Viktor Prutyanov
<viktor.prutyanov@xxxxxxxxxxxx> wrote:
+ amlogic,fifo-threshold:
+ description: TX FIFO threshold
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 127
I tried to make sense of this property but I don't understand it yet
(even after reading the second patch in this series).
A "FIFO" (in my own words) is some physical property of the IR
transmitter in these Amlogic SoCs.
So for one specific SoC there can only be one FIFO size, not a range (0..127).

What about a value of 0: my understanding is that this means that
there's no FIFO. Will this hardware still work in that case?

From reading the driver code it seems to me that the FIFO size is 128.
The driver can use fewer FIFO entries if it wants, but this must not
affect the dt-bindings (as these describe the hardware - they don't
"configure" the driver).
If you look at arch/arm64/boot/dts/amlogic/meson-g12.dtsi you'll find
toddr_a, toddr_b and toddr_c there:
All three of them use identical circuitry internally, except that
toddr_a has a bigger FIFO size than the other two. Using a FIFO size
of 256 for toddr_a is not correct as it's an improper description of
the toddr_a hardware (that said, the driver can still decide that it
only wants to use 256 FIFO entries).

In general, a FIFO threshold is not about how much of the FIFO you use overall, but how often and/or urgently you tend to it. If the only thing that matters is minimising CPU overhead then the optimum choice is to wait until the FIFO is entirely full/empty before taking action to drain/refill it. However, that necessarily creates a pause in reception/transmission for the time it takes to respond to the empty/full interrupt, hence why many FIFOs also implement a threshold interrupt for cases when uninterrupted communication is more desirable than absolutely minimising interrupts. Typically those are set to fire at some point shortly *before* the FIFO becomes completely full/empty, to leave enough remaining buffer for communication to continue during that time window until the ISR actually gets to respond.

That said, I'm also doubtful about this particular case. If the physical FIFO depth does actually vary between SoCs, that should be known by the driver and implicit in the compatible string, definitely not hidden in a tangential property. Otherwise, it's not apparent how this makes sense to configure statically on a per-SoC or per-board basis. If anything it would depend on the transmission rate of whatever IR protocol the user wishes to use at any given time AFAICS. If it's not sufficient for the driver to simply assume, say, an 80% threshold as "good enough", then presumably it has enough information about the clock rate and/or the parameters of the given Tx request to implement a slightly cleverer heuristic. If it's desirable to tweak the specific driver behaviour in cases where the user does know better, then by all means make that a module parameter, but it's not something which belongs in DT.