On Mon, Jun 10, 2019 at 8:07 PM Marek Vasut marex@denx.de wrote:
On 6/10/19 8:07 AM, Ley Foon Tan wrote: [...]
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
ulong rate;+#if CONFIG_IS_ENABLED(CLK)
rate = clk_get_rate(&i2c->clk);Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use.
Not in .probe() , in set_bus_speed().
My patch is doing it in designware_i2c_set_bus_speed() already. We can't get clock rate in __dw_i2c_set_bus_speed(), because this function doesn't have struct udevice.
include/i2c.h struct dm_i2c_ops {} :
388 /** 389 * set_bus_speed() - set the speed of a bus (optional) 390 * 391 * The bus speed value will be updated by the uclass if this function 392 * does not return an error. This method is optional - if it is not 393 * provided then the driver can read the speed from 394 * dev_get_uclass_priv(bus)->speed_hz 395 * 396 * @bus: Bus to adjust 397 * @speed: Requested speed in Hz 398 * @return 0 if OK, -EINVAL for invalid values 399 */ 400 int (*set_bus_speed)(struct udevice *bus, unsigned int speed);
There's struct udevice right there ^
I'm confused now.
.set_bus_speed = designware_i2c_set_bus_speed and my patch is doing get clock rate in .set_bus_speed callback already.
static const struct dm_i2c_ops designware_i2c_ops = { .xfer = designware_i2c_xfer, .probe_chip = designware_i2c_probe_chip, .set_bus_speed = designware_i2c_set_bus_speed, };
Regards Ley Foon