Hi Mike,
On Mon, Jan 18, 2010 at 11:52:44PM -0500, Mike Frysinger wrote:
> The ad7879 driver is using the old bus method of only supporting one at a
> time (I2C or SPI). So refactor it like the other input drivers that
> support multiple busses simultaneously.
>
Nice patch, thank you. A couple of suggestions though:
> -
> struct ad7879 {
> - bus_device *bus;
> + struct ad7879_bus_ops bops;
Could you change it to 'const struct ad7879_bus_ops *bops;' and add a
separate pointer to private transport data so that bus ops are truly
shared between instances (if any).
> struct input_dev *input;
> struct work_struct work;
> struct timer_list timer;
Also I dont see the reason for having the following typedefs:
> +
> +typedef int (ad7879_read_t) (void *bus_data, u8 reg);
> +typedef int (ad7879_multi_read_t) (void *bus_data, u8 first_reg, u8 count, u16 *buf);
> +typedef int (ad7879_write_t) (void *bus_data, u8 reg, u16 val);
> +
> +struct ad7879_bus_ops {
> + void *bus_data;
> + int irq;
> + ad7879_read_t *read;
> + ad7879_multi_read_t *multi_read;
> + ad7879_write_t *write;
> +};
> +
> +int ad7879_disable(struct device *dev);
> +int ad7879_enable(struct device *dev);
> +int ad7879_probe(struct device *dev, struct ad7879_bus_ops *bops, u8 devid, u16 bustype);
> +int ad7879_remove(struct device *dev);
Hmm, I liked when these accepted "struct ad7879" better.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
