[PATCH v3 RFC] i2c: rk3x: handle dynamic clock rate changes correctly

[Doug Anderson]

Max,

On Tue, Sep 16, 2014 at 4:13 PM, Max Schwarz <max.schwarz at online.de> wrote:
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 93cfc83..c13fabc 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -97,6 +97,8 @@ struct rk3x_i2c {
>         /* Hardware resources */
>         void __iomem *regs;
>         struct clk *clk;
> +       struct notifier_block clk_rate_nb;
> +       unsigned long clk_freq;

Is it just me, or is "clk_freq" a write-only member of this structure?
 If it is, perhaps it could be removed.

I guess if it were not write only I'd expect it would be used in
rk3x_i2c_set_scl_rate(), but it's not.


> +static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate)
> +{
> +       unsigned long i2c_rate = clk_get_rate(i2c->clk);
> +       unsigned int div;
> +       int ret;
> +
> +       ret = rk3x_i2c_calc_div(i2c_rate, scl_rate, &div);
> +
> +       WARN_ONCE(ret != 0, "Could not reach desired SCL freq %lu", scl_rate);
> +
> +       /*
> +        * We might be called from rk3x_i2c_probe or from the clk rate change
> +        * notifier. In that case, the i2c clk is not running. So enable it
> +        * explicitly here during the register write.

It's now in _both_ cases that the i2c clk is not running, right?
...so we could just remove this comment totally?  Enabling a clock
around an i2c_write() doesn't seem incredibly noteworthy and it's now
consistent among the two callers...