linux-kernel - Re: [PATCH] firmware: arm_scmi: get only min/max clock rates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZqkCwk4jrLUm20in@pluto>
Date: Tue, 30 Jul 2024 16:12:02 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Etienne Carriere <etienne.carriere@...s.st.com>
Cc: linux-kernel@...r.kernel.org, Sudeep Holla <sudeep.holla@....com>,
	Cristian Marussi <cristian.marussi@....com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, arm-scmi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH] firmware: arm_scmi: get only min/max clock rates

On Mon, Jul 29, 2024 at 08:53:06AM +0200, Etienne Carriere wrote:
> Remove limitation of 16 clock rates max for discrete clock rates
> description.
> 

Hi Etienne,

> Driver clk-scmi.c in only interested in the min and max clock rates.
> Get these by querying the first and last discrete rates with SCMI
> clock protocol message ID CLK_DESCRIBE_RATES since the SMCI clock
> protocol specification states that rates enumerated by this
> command are to be enumerated in "numeric ascending order" [1].
> 

While I understand the positive optinization of not having to query the
full list of clock domains in order to retrieve just min/max (that are
the only interesting clocks for CLK framework), and the removal of the
artificial 16 clocks limit, I would NOT drop in its entirety the original
well tested code used to query the full list (based on iterators), because
it will be most probably needed anyway in the future to implement
determine_rate via bisection (if I udnerstood correctly what you meant
in our offline chat) and because it would be needed anyway for testing
purposes.

IOW, why dont you do exactly what you did BUT within a new distinct
scmi_describe_min_max_rates() helper, WHILE keeping the original code
scmi_clock_describe_rates_get() unchanged, and start calling your new
method at init to retrieve only max/min ?

So that we can attach anyway (now or then) a new 'lazy' ops to the old
method and use it to retrieve the full list of clocks when needed (for
testing or whatever).

Thanks,
Cristian