Max,
On Tue, Sep 23, 2014 at 1:27 AM, Max Schwarz <max.schwarz at online.de> wrote:
> The i2c input clock can change dynamically, e.g. on the RK3066 where
> pclk_i2c0 and pclk_i2c1 are connected to the armclk, which changes
> rate on cpu frequency scaling.
>
> Until now, we incorrectly called clk_get_rate() while holding the
> i2c->lock in rk3x_i2c_xfer() to adapt to clock rate changes.
> Thanks to Huang Tao for reporting this issue.
>
> Do it properly now using the clk notifier framework. The callback
> logic was taken from i2c-cadence.c.
>
> Signed-off-by: Max Schwarz <max.schwarz at online.de>
> Tested-by: Max Schwarz <max.schwarz at online.de> on RK3188
> Tested-by: Doug Anderson <dianders at chromium.org> on RK3288
> (dynamic rate changes are untested!)
> ---
>
> This is based on Wolframs' i2c/for-current branch, since that
> includes the recent divisor fix by Addy Ke (b4a7bd7a38).
>
> Doug, I'm keeping your Tested-By since the changes should not touch
> the static clock case, but you might want to confirm that the
> Reviewed-By still stands.
No problem. I just re-tested your v5 and it still boots. You can add
my Reviewed-by back though I have one nit below.
> Changes since v4:
> - fixed a bug in rk3x_i2c_clk_notifier_cb() which would feed
> the input clock frequency as the desired SCL frequency into
> rk3x_i2c_set_scl_frequency(i2c, scl_rate). Rename & change to
> rk3x_i2c_adapt_div(i2c, clk_rate) to make things clear.
Whew, glad you caught this! Since the notifier block wasn't used on
rk3288 which I'm using, I didn't spend nearly enough time check that
part of the code...
> +static int rk3x_i2c_clk_notifier_cb(struct notifier_block *nb, unsigned long
> + event, void *data)
> +{
> + struct clk_notifier_data *ndata = data;
> + struct rk3x_i2c *i2c = container_of(nb, struct rk3x_i2c, clk_rate_nb);
> + unsigned int div;
> +
> + switch (event) {
> + case PRE_RATE_CHANGE:
> + {
Now that you're not declaring any variables here, you don't need
braces in this case.
-Doug
