On 15 December 2015 at 20:15, Trent Piepho <tpiepho@xxxxxxxxxxxxxx> wrote:
> 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.
>
Should we go as far as removing the read_regs ops ? Doing so will
specialize "mcp23s08_probe_one" for the i2c version of the chip. If we
ever want to integrate back the SPI part of the linux driver, leaving
the op has a minimal cost while enabling us to keep mcp23s08_probe_one
in sync with the linux's version.
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
