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

Reply via email to