[U-Boot] [PATCH v3 01/10] dm: i2c: Add a uclass for I2C

[Masahiro Yamada <yamada.m@jp.panasonic.com>]

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