Re: [PATCH v2 1/3] clk: samsung: exynos850: Propagate SPI IPCLK rate change

From: Tudor Ambarus <tudor.ambarus@linaro.org>
To: Sam Protsenko <semen.protsenko@linaro.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/3] clk: samsung: exynos850: Propagate SPI IPCLK rate change
Date: Mon, 29 Jan 2024 17:46:41 +0000	[thread overview]
Message-ID: <99b7501d-6780-4227-8558-488b1b9da758@linaro.org> (raw)
In-Reply-To: <20240125013858.3986-2-semen.protsenko@linaro.org>

Hi, Sam,

On 1/25/24 01:38, Sam Protsenko wrote:
> Which should cover all possible applications of SPI bus. Of course,
> setting SPI frequency to values as low as 500 kHz will also affect the
> common bus dividers (dout_apm_bus or dout_peri_ip), which in turn
> effectively lowers the rates for all leaf bus clocks derived from those
> dividers, like HSI2C and I3C clocks. But at least it gives the board
> designer a choice, whether to keep all clocks (SPI/HSI2C/I3C) at high
> frequencies, or make all those clocks have lower frequencies. Not
> propagating the rate change to those common dividers would limit this
> choice to "only high frequencies are allowed for SPI/HSI2C/I3C" option,
> making the common dividers useless. This decision follows the "Worse is
> better" approach, relying on the users/engineers to know the system
> internals when working with such low-level features, instead of trying
> to account for all possible use-cases.

Depending on clock frequencies in DT and on the order of probe, one may
end up with SPI changing the frequency for I3C for example, there's no
protection on that. The more conservative approach, to which I lean, is
to propagate the clock just to the first divider, which is dedicated to
the end note, thus you'll avoid such problems. I think this fine tuning
that you allow is more suitable for downstream. Maybe this is more of a
personal preference, I don't know. Curious what others think.

The patch is looking fine though, and if the approach is considered
acceptable:
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>