On Fri, Oct 16, 2009 at 03:22, Dmitry Torokhov wrote: > On Fri, Oct 16, 2009 at 02:57:15AM -0400, Mike Frysinger wrote: >> On Fri, Oct 16, 2009 at 01:39, Mike Frysinger wrote: >> > On Fri, Oct 16, 2009 at 00:36, Dmitry Torokhov wrote: >> >> On Wed, Oct 14, 2009 at 06:54:40AM -0400, Mike Frysinger wrote: >> >>> +struct device; >> >>> +struct adxl34x; >> >>> +typedef int (adxl34x_read_t) (struct device *, unsigned char); >> >>> +typedef int (adxl34x_read_block_t) (struct device *, unsigned char, int, unsigned char *); >> >>> +typedef int (adxl34x_write_t) (struct device *, unsigned char, unsigned char); >> >>> + >> >>> +void adxl34x_disable(struct adxl34x *ac); >> >>> +void adxl34x_enable(struct adxl34x *ac); >> >>> +int adxl34x_probe(struct adxl34x **pac, struct device *dev, u16 bus_type, >> >>> + int irq, int fifo_delay_default, adxl34x_read_t read, >> >>> + adxl34x_read_block_t read_block, adxl34x_write_t write); >> >> >> >> Too many arguments... I think creating "struct adxl34x_ops" is called >> >> for. >> > >> > guess i should do the same with the ad714x driver ? >> >> although looking at it, it'd only combine 4 args into 1 (so there'd be >> 5 instead of 8). i'd still have to pass the rest in as they're >> instance-specific. guess you still want the change though ? > > Yes, please. I still think it would be cleaner this way. i converted the bus pieces to a struct then: +struct adxl34x_bus_ops { + u16 bustype; + adxl34x_read_t *read; + adxl34x_read_block_t *read_block; + adxl34x_write_t *write; +}; i'll leave the rest of your feedback to be addressed by Michael since he has actual hardware to test with (and he wrote/understands the driver) -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