On Tue, Jan 19, 2010 at 03:26, Dmitry Torokhov wrote: > On Mon, Jan 18, 2010 at 11:52:44PM -0500, Mike Frysinger wrote: >> 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). i can make the struct containing only the funcs const, but not the struct that contains the irq/client since i dynamically assign the contents in the probe func > Also I dont see the reason for having the following typedefs: it made sense in the other drivers i copied this format from, and in an earlier version of the code, but they arent needed anymore, so i'll drop em >> +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. i toyed with that a bit, but the generated code was worse because the drvdata helpers are externals -mike -- 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