Re: [PATCH] spi: meson-spicc: save pow2 datarate between messages

From: Mark Brown <broonie@kernel.org>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org,
	Da Xue <da@libre.computer>
Subject: Re: [PATCH] spi: meson-spicc: save pow2 datarate between messages
Date: Wed, 10 Aug 2022 17:10:15 +0100	[thread overview]
Message-ID: <YvPYZ/0o/Xa+vDKa@sirena.org.uk> (raw)
In-Reply-To: <c7bfda2e-8135-fd2e-5d2b-5f547215bb18@baylibre.com>

[-- Attachment #1.1: Type: text/plain, Size: 1876 bytes --]

On Wed, Aug 10, 2022 at 05:51:33PM +0200, Neil Armstrong wrote:
> On 10/08/2022 16:49, Mark Brown wrote:

> > I don't recall the code for clock providers being that hard?  They're
> > generally pretty small, some of the ASoC CODEC drivers did something
> > similar.

> Seems over-engineering to me, but I can explore this path if it's the best
> route to follow.

Please.

> > > had an open-coded function which perfectly worked before.

> > Except in the cases it didn't...

> It did but wasn't generic enough to take the new clock path introduced
> in the new SoCs.

Right, it's leaving landmines lying around - it's hard for me to be
confident that we couldn't also get another surprise due to a new test
case exercising the code differently somehow, never mind the fragility
of the code.

> > > I'm perfectly OK to remove the CCF driver for the legacy clock path
> > > and return back to the old open coded calculation since it perfectly
> > > worked and stop using the legacy clock path for new SoCs since it would
> > > never be selected anyway...

> > It does seem better to go the clock provider route TBH.

> I'm afraid this won't fix the problem since CCF won't set the clock again
> if the rate is already ok in it's cache, we'd still need to save the divider
> value and restore it after the reset as I did on this exact patch.

The goal here is to ensure that the clock framework's idea of what the
programmed configuration is and the SPI driver's idea of what that is
can't get out of sync - instead of saving the value by virtue of reading
it back out of the hardware at some specific point and hoping that
there's never any changes triggered by the clock framework between the
save and restore the driver is getting notified whenever there's a
change being made and can update the cache then.  That way we ensure
we can't miss any cases and things are robust.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

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