On Wed, May 19, 2010 at 06:45, Michael Hennerich wrote:
> +int ad7991_5_9_set_scan_mode(struct ad799x_state *st, unsigned mask)
> +{
> + return i2c_smbus_write_byte(st->client,
> + st->config | (mask << AD799X_CHANNEL_SHIFT));
> +}
> +
> +int ad7992_3_4_set_scan_mode(struct ad799x_state *st, unsigned mask)
> +{
> +
> + return ad799x_i2c_write8(st, AD799X_CONF_REG,
> + st->config | (mask << 4));
> +}
> +
> +int ad7997_8_set_scan_mode(struct ad799x_state *st, unsigned mask)
> +{
> + return ad799x_i2c_write16(st, AD799X_CONF_REG,
> + st->config | (mask << 4));
> +}
> +
> +int ad799x_set_scan_mode(struct ad799x_state *st, unsigned mask)
> +{
> + int ret;
> +
> + if (st->chip_info->ad799x_set_scan_mode != NULL) {
> + ret = st->chip_info->ad799x_set_scan_mode(st, mask);
> + return (ret > 0) ? 0 : ret;
> + }
> +
> + return 0;
> +}
shouldnt these all be static ?
> +/* ad799x and max1368 tested - rest from data sheet */
> +static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
> + .ad799x_set_scan_mode = (void*) ad7991_5_9_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7991_5_9_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7991_5_9_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7992_3_4_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7992_3_4_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7992_3_4_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7997_8_set_scan_mode,
> + .ad799x_set_scan_mode = (void*) ad7997_8_set_scan_mode,
shouldnt the prototype and/or function defintion be fixed instead of
using a (void*) cast ?
> +static int ad799x_initial_setup(struct ad799x_state *st)
> +{
> + st->config = st->chip_info->default_config;
> +
> + return ad799x_set_scan_mode(st, 0);
> +}
seems silly to have such a small local func that gets used only once
to not be inlined in the source ? if it does stay separate, then it
probably should have __devinit added to it like the probe func that
calls it.
> +static int __devinit ad799x_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +static int ad799x_remove(struct i2c_client *client)
since the prob func has __devinit, shouldnt the remove func have __devexit ?
> + .remove = ad799x_remove,
which would then need __devexit_p() ...
-mike
_______________________________________________
Uclinux-dist-devel mailing list
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel