mbox series

[0/4] Getting rid of the reset controller in i2s-tdm

Message ID 20211016105354.116513-1-frattaroli.nicolas@gmail.com
Headers show
Series Getting rid of the reset controller in i2s-tdm | expand

Message

Nicolas Frattaroli Oct. 16, 2021, 10:53 a.m. UTC 
Hello,

after some discussion with Heiko on IRC, he has admitted to me
that the rockchip,cru property, and its corresponding half a reset
controller in the driver, is weighing heavily on his mind.

The background is that if the lrck only uses one clock for both rx
and tx direction, then according to the downstream driver, the rx
and tx resets should be asserted at roughly the same time to keep
things in sync.

Since there is no existing kernel way of doing this, the driver
would manually write to the CRU's registers to achieve this,
violating abstractions.

We've agreed that an atomic bulk reset API would be the best way to
achieve what it does in a clean fashion. The details of such an API
have yet to be worked out by me, but as it turns out, this is not
a pressing need.

During my investigation, I noticed that I can simply drop the
synchronised reset for now and assert the two resets manually one
after the other, and deassert them in the same manner.

For the case I care about, which is audio playback, this seems to
work just fine. Should someone actually find a case where this
causes a problem, it should be fixed with an atomic bulk reset API.

Patch 1 removes the direct CRU writing stuff from the i2s-tdm driver.

Patch 2 drops the rockchip,cru property from the bindings; they have
not yet been in a kernel release, so as far as I know, we can still
change them with no regard for backwards compatibility.

Patch 3 adds the rk356x i2s1 node without the rockchip,cru property.

Patch 4 adds the analog audio output on Quartz64, included here for
Heiko's convenience.

Regards,
Nicolas Frattaroli

Nicolas Frattaroli (4):
  ASoC: rockchip: i2s-tdm: Strip out direct CRU use
  ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip,cru property
  arm64: dts: rockchip: Add i2s1 on rk356x
  arm64: dts: rockchip: Add analog audio on Quartz64

 .../bindings/sound/rockchip,i2s-tdm.yaml      |  16 ---
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   |  31 ++++-
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  25 ++++
 sound/soc/rockchip/rockchip_i2s_tdm.c         | 126 +++---------------
 4 files changed, 76 insertions(+), 122 deletions(-)

Comments

Mark Brown Oct. 17, 2021, 11:36 a.m. UTC | #1 
On Sat, 16 Oct 2021 12:53:49 +0200, Nicolas Frattaroli wrote:
> after some discussion with Heiko on IRC, he has admitted to me
> that the rockchip,cru property, and its corresponding half a reset
> controller in the driver, is weighing heavily on his mind.
> 
> The background is that if the lrck only uses one clock for both rx
> and tx direction, then according to the downstream driver, the rx
> and tx resets should be asserted at roughly the same time to keep
> things in sync.
> 
> [...]

Applied, thanks!

[1/4] ASoC: rockchip: i2s-tdm: Strip out direct CRU use
      commit: d6365d0f0a03c1feb28d86dfd192972ddc647013
[2/4] ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip,cru property
      commit: 4e52cb9e2c22c9d860910794c82461064baadd9f

Best regards,
Heiko Stuebner Oct. 17, 2021, 12:36 p.m. UTC | #2 
On Sat, 16 Oct 2021 12:53:49 +0200, Nicolas Frattaroli wrote:
> after some discussion with Heiko on IRC, he has admitted to me
> that the rockchip,cru property, and its corresponding half a reset
> controller in the driver, is weighing heavily on his mind.
> 
> The background is that if the lrck only uses one clock for both rx
> and tx direction, then according to the downstream driver, the rx
> and tx resets should be asserted at roughly the same time to keep
> things in sync.
> 
> [...]

Applied, thanks!

[3/4] arm64: dts: rockchip: Add i2s1 on rk356x
      commit: ef5c913570040df1955dd49cea221783468faeaf
[4/4] arm64: dts: rockchip: Add analog audio on Quartz64
      commit: 1938b585ed19bb01969b4e923664db88c5ee8798

Best regards,