Hi Simon,
A little more comments from me.
On Mon, 24 Nov 2014 11:57:15 -0700
Simon Glass <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
