Hi Heiko,
On 2/27/24 17:46, Heiko Stuebner wrote:
From: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
The rockchip,trcm-sync-tx-only property is at this time only documented
for the tdm variant of Rockchip i2s controllers.
While there was a series [0] adding code and binding for the property,
it doesn't seem to have gone forward back in 2021.
So for now fix the devicetree check by removing the property from rk3588
i2s controllers until support for it gets merged.
It seems like tx-only should be supported if the dai_link has a
symmetric rate, c.f.
https://elixir.bootlin.com/linux/latest/source/sound/soc/rockchip/rockchip_i2s.c#L455
is doing the same as the patch from 3 years ago[1] was trying to do,
only in the probe.
[1]
https://patchwork.kernel.org/project/linux-rockchip/patch/1629796734-4243-4-git-send-email-sugar.zhang@xxxxxxxxxxxxxx/
Considering that the RK3588 doesn't have RX only support according to
the documentation of that register....
However, I have no clue how the dai_link would get the symmetrical rate
set, because the only place I could see it set is in set_link_flags if
SND_SOC_TPLG_LNK_FLGBIT_SYMMETRIC_RATES if set in the flag mask and the
flags, but my grep-fu returned nothing setting this flag anywhere... so
maybe that's just dead code?
In any case, with the current code:
I2S_CKR_TRCM_TXRX is put into I2S_CKR register, regardless of that DT
property. While the naming seems to suggest TX+RX should be possible,
its value is 0 (offset 28) and in the TRM it says:
2'b00: Generates LRCK for TX only.
Soooo... we are essentially in tx-only mode today with or without that
property. Some things to fix later on :)
Which made me look for the same thing for the i2s_tdm driver and the
same mistake is made, though here there's no support for tx-only or
rx-only according to the TRM, and it is required to write 0b01 (offset
28)... which we do by abusing the tx-only DT property which writes
TRCM_TX whose value is 1 (offset 28). Considering that this is stored in
clk_trcm member in the struct and that we do checks on that member to
know in which situation we are (txrx, tx, rx), this seems very incorrect
to me as we would configure it in the only mode it knows (txrx) but make
the driver assume tx-only is selected. One more thing to fix later on :)
In any case, this patch is for matching the dt-bindings so there's no
reason to not merge this, so:
Reviewed-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
Thanks,
Quentin
