On 2018-08-17 20:29, Mark Brown wrote:
On Fri, Aug 17, 2018 at 04:06:06PM +0530, dkota@xxxxxxxxxxxxxx wrote:
Could you please clarify on below query.
I'm not seeing any further questions in your e-mail?
My bad; i mean, you to comment on below explanation on using
cur_speed_hz instead of clk_get_rate().
+ mas->cur_speed_hz = spi_slv->max_speed_hz;
Why can't you use clk_get_rate() instead? Or call clk_set_rate() with
the rate you want the master clk to run at and then divide that down
from there?
> Not sure I follow, the intention is to run the controller clock based on
> the slave's max frequency.
That's good. The problem I see is that we have to specify the max
frequency in the controller/bus node, and also in the child/slave
node.
It should only need to be specified in the slave node, so making the
cur_speed_hz equal the max_speed_hz is problematic. The current speed
of
the master should be determined by calling clk_get_rate().
We don't require that the slaves all individually set a speed since it
gets a bit redundant, it should be enough to just use the default the
controller provides. A bigger problem with this is that the driver
will
never see a transfer which doesn't explicitly have a speed set as the
core will ensure something is set, open coding this logic in every
driver would obviously be tiresome.
clock_get_rate() will returns the rate which got set as per the clock
plan(which was the rounded up frequency) which can be less than or equal
to the requested frequency. For that reason using the cur_speed_hz to
store the requested frequency and skip clock configuration for the
consecutive transfers with same frequency.
I have uploaded the new patchset addressing all these review comments.
Please review it.
--Dilip
