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
(just reading this patch out of passing curiosity...)
On 2021-07-17 20:57, Martin Blumenstingl wrote:
On Fri, Jul 16, 2021 at 4:45 PM Viktor Prutyanov
I tried to make sense of this property but I don't understand it yet
+ description: TX FIFO threshold
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 127
(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.