All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Cc: linux-rockchip@lists.infradead.org, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/4] ASoC: rockchip: i2s-tdm: Strip out direct CRU use
Date: Sat, 16 Oct 2021 17:50:22 +0200	[thread overview]
Message-ID: <1782571.1Dz21PRzoM@phil> (raw)
In-Reply-To: <20211016105354.116513-2-frattaroli.nicolas@gmail.com>

Am Samstag, 16. Oktober 2021, 12:53:50 CEST schrieb Nicolas Frattaroli:
> In cases where both rx and tx lrck are synced to the same source,
> the resets for rx and tx need to be triggered simultaneously,
> according to the downstream driver.
> 
> As there is no reset API to atomically bulk (de)assert two resets
> at once, what the driver did was implement half a reset controller
> specific to Rockchip, which tried to write the registers for the
> resets within one write ideally or several writes within an irqsave
> section.
> 
> This of course violates abstractions quite badly. The driver should
> not write to the CRU's registers directly.
> 
> In practice, for the cases I tested the driver with, which is audio
> playback, replacing the synchronised asserts with just individual
> ones does not seem to make any difference.
> 
> If it turns out that this breaks something in the future, it should
> be fixed through the specification and implementation of an atomic
> bulk reset API, not with a CRU hack.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Cc: linux-rockchip@lists.infradead.org, alsa-devel@alsa-project.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] ASoC: rockchip: i2s-tdm: Strip out direct CRU use
Date: Sat, 16 Oct 2021 17:50:22 +0200	[thread overview]
Message-ID: <1782571.1Dz21PRzoM@phil> (raw)
In-Reply-To: <20211016105354.116513-2-frattaroli.nicolas@gmail.com>

Am Samstag, 16. Oktober 2021, 12:53:50 CEST schrieb Nicolas Frattaroli:
> In cases where both rx and tx lrck are synced to the same source,
> the resets for rx and tx need to be triggered simultaneously,
> according to the downstream driver.
> 
> As there is no reset API to atomically bulk (de)assert two resets
> at once, what the driver did was implement half a reset controller
> specific to Rockchip, which tried to write the registers for the
> resets within one write ideally or several writes within an irqsave
> section.
> 
> This of course violates abstractions quite badly. The driver should
> not write to the CRU's registers directly.
> 
> In practice, for the cases I tested the driver with, which is audio
> playback, replacing the synchronised asserts with just individual
> ones does not seem to make any difference.
> 
> If it turns out that this breaks something in the future, it should
> be fixed through the specification and implementation of an atomic
> bulk reset API, not with a CRU hack.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Cc: linux-rockchip@lists.infradead.org, alsa-devel@alsa-project.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] ASoC: rockchip: i2s-tdm: Strip out direct CRU use
Date: Sat, 16 Oct 2021 17:50:22 +0200	[thread overview]
Message-ID: <1782571.1Dz21PRzoM@phil> (raw)
In-Reply-To: <20211016105354.116513-2-frattaroli.nicolas@gmail.com>

Am Samstag, 16. Oktober 2021, 12:53:50 CEST schrieb Nicolas Frattaroli:
> In cases where both rx and tx lrck are synced to the same source,
> the resets for rx and tx need to be triggered simultaneously,
> according to the downstream driver.
> 
> As there is no reset API to atomically bulk (de)assert two resets
> at once, what the driver did was implement half a reset controller
> specific to Rockchip, which tried to write the registers for the
> resets within one write ideally or several writes within an irqsave
> section.
> 
> This of course violates abstractions quite badly. The driver should
> not write to the CRU's registers directly.
> 
> In practice, for the cases I tested the driver with, which is audio
> playback, replacing the synchronised asserts with just individual
> ones does not seem to make any difference.
> 
> If it turns out that this breaks something in the future, it should
> be fixed through the specification and implementation of an atomic
> bulk reset API, not with a CRU hack.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Cc: linux-rockchip@lists.infradead.org, alsa-devel@alsa-project.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] ASoC: rockchip: i2s-tdm: Strip out direct CRU use
Date: Sat, 16 Oct 2021 17:50:22 +0200	[thread overview]
Message-ID: <1782571.1Dz21PRzoM@phil> (raw)
In-Reply-To: <20211016105354.116513-2-frattaroli.nicolas@gmail.com>

Am Samstag, 16. Oktober 2021, 12:53:50 CEST schrieb Nicolas Frattaroli:
> In cases where both rx and tx lrck are synced to the same source,
> the resets for rx and tx need to be triggered simultaneously,
> according to the downstream driver.
> 
> As there is no reset API to atomically bulk (de)assert two resets
> at once, what the driver did was implement half a reset controller
> specific to Rockchip, which tried to write the registers for the
> resets within one write ideally or several writes within an irqsave
> section.
> 
> This of course violates abstractions quite badly. The driver should
> not write to the CRU's registers directly.
> 
> In practice, for the cases I tested the driver with, which is audio
> playback, replacing the synchronised asserts with just individual
> ones does not seem to make any difference.
> 
> If it turns out that this breaks something in the future, it should
> be fixed through the specification and implementation of an atomic
> bulk reset API, not with a CRU hack.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



  reply	other threads:[~2021-10-16 15:51 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-16 10:53 [PATCH 0/4] Getting rid of the reset controller in i2s-tdm Nicolas Frattaroli
2021-10-16 10:53 ` Nicolas Frattaroli
2021-10-16 10:53 ` Nicolas Frattaroli
2021-10-16 10:53 ` Nicolas Frattaroli
2021-10-16 10:53 ` [PATCH 1/4] ASoC: rockchip: i2s-tdm: Strip out direct CRU use Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 15:50   ` Heiko Stuebner [this message]
2021-10-16 15:50     ` Heiko Stuebner
2021-10-16 15:50     ` Heiko Stuebner
2021-10-16 15:50     ` Heiko Stuebner
2021-10-16 10:53 ` [PATCH 2/4] ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip, cru property Nicolas Frattaroli
2021-10-16 10:53   ` [PATCH 2/4] ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip,cru property Nicolas Frattaroli
2021-10-16 10:53   ` [PATCH 2/4] ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip, cru property Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 15:47   ` Heiko Stuebner
2021-10-16 15:47     ` [PATCH 2/4] ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip,cru property Heiko Stuebner
2021-10-16 15:47     ` [PATCH 2/4] ASoC: dt-bindings: rockchip: i2s-tdm: Drop rockchip, cru property Heiko Stuebner
2021-10-16 15:47     ` Heiko Stuebner
2021-10-16 10:53 ` [PATCH 3/4] arm64: dts: rockchip: Add i2s1 on rk356x Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 10:53 ` [PATCH 4/4] arm64: dts: rockchip: Add analog audio on Quartz64 Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-16 10:53   ` Nicolas Frattaroli
2021-10-17 11:36 ` (subset) [PATCH 0/4] Getting rid of the reset controller in i2s-tdm Mark Brown
2021-10-17 11:45   ` Mark Brown
2021-10-17 11:45   ` Mark Brown
2021-10-17 11:45   ` Mark Brown
2021-10-17 11:45   ` Mark Brown
2021-10-17 11:36   ` Mark Brown
2021-10-17 11:36   ` Mark Brown
2021-10-17 11:36   ` Mark Brown
2021-10-17 12:36 ` Heiko Stuebner
2021-10-17 12:36   ` Heiko Stuebner
2021-10-17 12:36   ` Heiko Stuebner
2021-10-17 12:36   ` Heiko Stuebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1782571.1Dz21PRzoM@phil \
    --to=heiko@sntech.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=frattaroli.nicolas@gmail.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=p.zabel@pengutronix.de \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.