Re: [PATCH 2/5] i2c: rcar: get clock rate only once and simplify calculation

Hi Magnus

Thanks for your comments

On Wed, 11 Sep 2013, Magnus Damm wrote:

Hi Guennadi,

On Tue, Sep 10, 2013 at 12:55 AM, Guennadi Liakhovetski
[off-list ref] wrote:

quoted

There is no need to repeatedly query clock frequency, where it is not
expected to change. The complete loop can also trivially be replaced with
a simple division. A further loop below the one, being simplified, could
also be replaced, but that would get more complicated.

Signed-off-by: Guennadi Liakhovetski <redacted>

Thanks for your efforts. What makes you think that you should assume
that the clock frequency doesn't change?

Sorry, maybe I didn't explain well enough in the patch description. 
Please, have a look at the patch. It removes repeated clock rate readings 
inside a tight loop. I'm aware that the clock rate can change, but there 
should be a way to make sure, that beginning now and until a certain later 
point of time the rate doesn't change. Presumably this should be done by 
locking the clock. I'm not sure this is currently supported by the clock 
API. Neither clk_get(), nor clk_prepare() nor clk_enable() seem to have 
the semantics of locking the clock's frequency. Maybe the one who actually 
prepared the clock should become its owner, but I don't think this is 
currently the expected behaviour. In either case, I don't think calling 
clk_get_rate() repeatedly inside _that_ loop makes sense - the rate can 
anyway be changed after the loop, so, it doesn't add any security.

Thanks
Guennadi

As you already know, we want drivers to be reusable across multiple
SoCs. Assuming it that it would be fixed based on one particular SoC
doesn't guarantee that that's the case on other SoCs. Which SoCs did
you take into consideration?

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/