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