Add Addy to make sure he get this mail, it's wrong mail cc address
in Max's mail, and Addy didn't subscribe linux-rockchip mailing list at
that time.
On 09/16/2014 03:06 AM, Max Schwarz 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
> (dynamic rate changes are untested!)
> ---
>
> Changes since v1:
> - make sure the i2c input clock is active during prescaler
> register write by explicitly enabling/disabling it in
> rk3x_i2c_set_scl_frequency(). Bug found by Addy Ke.
>
> Can somebody with access to a RK3066 test this or tell me how to
> produce dynamic i2c clk rate changes on my RK3188?
>
> I guess the only good way to test this is with a logic analyzer
> or I2C sniffer during frequency changes.
>
> Cheers,
> Max
>
> drivers/i2c/busses/i2c-rk3x.c | 139 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 131 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 93cfc83..4c35526 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;
>
> /* Settings */
> unsigned int scl_frequency;
> @@ -428,18 +430,127 @@ out:
> return IRQ_HANDLED;
> }
>
> -static void rk3x_i2c_set_scl_rate(struct rk3x_i2c *i2c, unsigned long scl_rate)
> +/**
> + * Calculate divider value for desired SCL frequency
> + *
> + * @clk_rate: I2C input clock rate
> + * @scl_rate: Desired SCL rate
> + * @div: Divider output
> + *
> + * Return: 0 on success, -EINVAL on unreachable SCL rate. In that case
> + * a best-effort divider value is returned in div.
> + **/
> +static int rk3x_i2c_calc_div(unsigned long clk_rate, unsigned long scl_rate,
> + unsigned int *div)
> {
> - unsigned long i2c_rate = clk_get_rate(i2c->clk);
> - unsigned int div;
> + /* Range checks */
> + if (scl_rate > clk_rate / 16) {
> + /* Return a best effort divider */
> + *div = 0;
> + return -EINVAL;
> + }
> +
> + if (scl_rate < clk_rate / (16 * 65536)) {
> + /* Return a best effort divider */
> + *div = 0xFFFF;
> + return -EINVAL;
> + }
>
> /* set DIV = DIVH = DIVL
> * SCL rate = (clk rate) / (8 * (DIVH + 1 + DIVL + 1))
> * = (clk rate) / (16 * (DIV + 1))
> */
> - div = DIV_ROUND_UP(i2c_rate, scl_rate * 16) - 1;
> + *div = DIV_ROUND_UP(clk_rate, scl_rate * 16) - 1;
>
> + return 0;
> +}
> +
> +/**
> + * Setup divider register for desired SCL frequency
> + *
> + * @scl_rate: Desired SCL Rate
> + **/
> +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;
> +
> + if (rk3x_i2c_calc_div(i2c_rate, scl_rate, &div) != 0) {
> + dev_warn(i2c->dev, "Could not reach desired SCL freq %lu Hz\n",
> + 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.
> + *
> + * Writing the register with halted clock crashes the system at least on
> + * RK3288.
> + */
> +
> + clk_enable(i2c->clk);
> i2c_writel(i2c, (div << 16) | (div & 0xffff), REG_CLKDIV);
> + clk_disable(i2c->clk);
> +}
> +
> +/**
> + * rk3x_i2c_clk_notifier_cb - Clock rate change callback
> + * @nb: Pointer to notifier block
> + * @event: Notification reason
> + * @data: Pointer to notification data object
> + *
> + * The callback checks whether a valid bus frequency can be generated after the
> + * change. If so, the change is acknowledged, otherwise the change is aborted.
> + * New dividers are written to the HW in the pre- or post change notification
> + * depending on the scaling direction.
> + *
> + * Code adapted from i2c-cadence.c.
> + *
> + * Return: NOTIFY_STOP if the rate change should be aborted, NOTIFY_OK
> + * to acknowedge the change, NOTIFY_DONE if the notification is
> + * considered irrelevant.
> + */
> +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);
> +
> + switch (event) {
> + case PRE_RATE_CHANGE:
> + {
> + unsigned int div;
> +
> + if (rk3x_i2c_calc_div(ndata->new_rate, i2c->scl_frequency,
> + &div) != 0) {
> + return NOTIFY_STOP;
> + }
> +
> + i2c->clk_freq = ndata->new_rate;
> +
> + /* scale up */
> + if (ndata->new_rate > ndata->old_rate)
> + rk3x_i2c_set_scl_rate(i2c, ndata->new_rate);
> +
> + return NOTIFY_OK;
> + }
> + case POST_RATE_CHANGE:
> + i2c->clk_freq = ndata->new_rate;
> +
> + /* scale down */
> + if (ndata->new_rate < ndata->old_rate)
> + rk3x_i2c_set_scl_rate(i2c, ndata->new_rate);
> + return NOTIFY_OK;
> + case ABORT_RATE_CHANGE:
> + /* scale up */
> + if (ndata->new_rate > ndata->old_rate)
> + rk3x_i2c_set_scl_rate(i2c, ndata->old_rate);
> + return NOTIFY_OK;
> + default:
> + return NOTIFY_DONE;
> + }
> }
>
> /**
> @@ -536,9 +647,6 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
>
> clk_enable(i2c->clk);
>
> - /* The clock rate might have changed, so setup the divider again */
> - rk3x_i2c_set_scl_rate(i2c, i2c->scl_frequency);
> -
> i2c->is_last_msg = false;
>
> /*
> @@ -724,16 +832,31 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> return ret;
> }
>
> + i2c->clk_rate_nb.notifier_call = rk3x_i2c_clk_notifier_cb;
> + ret = clk_notifier_register(i2c->clk, &i2c->clk_rate_nb);
> + if (ret != 0) {
> + dev_err(&pdev->dev, "Unable to register clock notifier\n");
> + goto err_clk;
> + }
> +
> + i2c->clk_freq = clk_get_rate(i2c->clk);
> +
> + clk_enable(i2c->clk);
> + rk3x_i2c_set_scl_rate(i2c, i2c->scl_frequency);
> + clk_disable(i2c->clk);
> +
> ret = i2c_add_adapter(&i2c->adap);
> if (ret < 0) {
> dev_err(&pdev->dev, "Could not register adapter\n");
> - goto err_clk;
> + goto err_clk_notifier;
> }
>
> dev_info(&pdev->dev, "Initialized RK3xxx I2C bus at %p\n", i2c->regs);
>
> return 0;
>
> +err_clk_notifier:
> + clk_notifier_unregister(i2c->clk, &i2c->clk_rate_nb);
> err_clk:
> clk_unprepare(i2c->clk);
> return ret;
