On Tue, 2015-12-15 at 18:28 +0100, Florian Vallee wrote: > + > +/* A given spi_device can represent up to eight mcp23sxx chips > + * sharing the same chipselect but using different addresses > + * (e.g. chips #0 and #3 might be populated, but not #1 or $2). > + * Driver data holds all the per-chip data. > + */ > +struct mcp23s08_driver_data { > + unsigned ngpio; > + struct mcp23s08 *mcp[8]; > + struct mcp23s08 chip[]; > +}; It doesn't look like anything uses this struct. > +static int > +mcp23008_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n) > +{ > + while (n--) { > + int ret = mcp23008_read(mcp, reg++); This could use mcp->ops->read(mcp, reg++) instead of mcp23008_read(). Which would make it exactly the same as this function: > +static int > +mcp23017_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n) > +{ > + while (n--) { > + int ret = mcp23017_read(mcp, reg++); > + if (ret < 0) > + return ret; > + *vals++ = ret; > + } > + > + return 0; > +} So just one read_regs fuction is needed. Which also would allow read_regs to be removed from the ops struct since it's the same for both chips. > + > +static const struct mcp23s08_ops mcp23008_ops = { > + .read = mcp23008_read, > + .write = mcp23008_write, > + .read_regs = mcp23008_read_regs, > +}; > + _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox