
Hi Simon,
A little more comments from me.
On Mon, 24 Nov 2014 11:57:15 -0700 Simon Glass sjg@chromium.org wrote:
+int i2c_set_bus_speed(struct udevice *bus, unsigned int speed) +{
- struct dm_i2c_ops *ops = i2c_get_ops(bus);
- struct dm_i2c_bus *i2c = bus->uclass_priv;
- int ret;
- if (ops->set_bus_speed) {
ret = ops->set_bus_speed(bus, speed);
if (ret)
return ret;
- }
- i2c->speed_hz = speed;
- return 0;
+}
This looks odd.
If each driver does not have .set_bus_speed handler, we cannot change the bus speed because changing the bus speed involves some hardware register(s) setting.
We should not change i2c->speed_hz without changing the actual speed.
I think the code should be:
if (ops->set_bus_speed) { ret = ops->set_bus_speed(bus, speed); if (ret) return ret; i2c->speed_hz = speed; }
+ /** + * set_bus_speed() - set the speed of a bus (optional) + * + * The bus speed value will be updated by the uclass if this function + * does not return an error. + * + * @bus: Bus to adjust + * @speed: Requested speed in Hz + * @return 0 if OK, -INVAL for invalid values
s/-INVAL/-EINVAL/
Best Regards Masahiro Yamada