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

Max Schwarz max.schwarz at online.de
Tue Sep 23 08:32:42 PDT 2014


Hi Doug,

Am Dienstag, 23. September 2014, 08:25:11 schrieb Doug Anderson:
> > 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.

Thanks!

> > 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...

Yeah, it just highlights we that we need some way to test this. Apart from my 
buggy code we also don't really know how the hw reacts to writing the divider 
in the middle of a transfer...

I'll try to write something that forces clock rate changes even on 
RK3188/RK3288.

> > +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.

Thanks, will remove them.

Cheers,
  Max



More information about the Linux-rockchip mailing list