Messages in this thread | | | From | Peter Korsgaard <> | Subject | Re: [PATCH] mcp23s08: support mcp23s17 variant | Date | Mon, 14 Feb 2011 19:29:50 +0100 |
| |
>>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
Hi,
>> - return (status < 0) ? status : rx[0]; >> + >> + if (mcp->type == MCP_TYPE_S17) { >> + tx[1] <<= 1; >> + >> + status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, 2); >> + return (status < 0) ? status : (rx[0] | (rx[1] << 8)); >> + } else { >> + status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, 1); >> + return (status < 0) ? status : rx[0]; >> + }
Grant> Rather than checking ->type for every transaction, would a set of Grant> callbacks for each type be better? It would probably have lower Grant> overhead and be simpler to maintain.
We could. I didn't do it because the 8bit and 16bit cases are very similar, and checking a simple integer is presumably a lot faster than the spi access - But I can rework the patch if you insists.
>> +++ b/include/linux/spi/mcp23s08.h >> @@ -2,21 +2,24 @@ >> /* FIXME driver should be able to handle IRQs... */ >> >> struct mcp23s08_chip_info { >> - bool is_present; /* true iff populated */ >> - u8 pullups; /* BIT(x) means enable pullup x */ >> + bool is_present; /* true if populated */ >> + unsigned pullups; /* BIT(x) means enable pullup x */ >> };
Grant> Unrelated whitespace changes.
I wouldn't call it unrelated. It's to ensure it still lines up after the s/u8/unsigned/.
-- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |