On 05/25/2016 07:52 AM, Jean Delvare wrote:
Hi Corey,
On Tue, 19 Jan 2016 11:09:37 -0600, minyard@xxxxxxx wrote:
From: Corey Minyard <cminyard@xxxxxxxxxx>
There was a lot of redundant code in the driver, basically one copy
for the polled interface and one for the interrupt-driven interface.
Only use the interrupt-driven interface and use a timer to check
the interface periodically and time things out.
This patch is simply too large and messy for me to consider reviewing
it. It needs to be split into smaller pieces. But honestly I don't see
that much redundancy in the driver, and as a matter of fact your patch
adds more lines than it removes.
Umm:
drivers/i2c/busses/i2c-i801.c | 582 +++++++++++++++++++++---------------------
1 file changed, 284 insertions(+), 298 deletions(-)
It's not a lot of lines, but it is a little smaller.
This is unfortunate because some of the changes would still be good to
have. In particular having a single call to i801_check_post() would be
a nice clean-up.
Yes, looking over this some more, several of the issues have been
addressed that these patches originally targeted.
This was originally part of a set of patches that allowed access to
I2C devices when the system couldn't schedule, primarily for
storing panic information and handling watchdogs at panic and
in kgdb. For that you have to event-drive it so you can poll.
I've abandoned those changes, but this small set looked
like it has some useful stuff.
Thanks for the review, I'll look at pulling out some of the good things
(and getting rid of hwpec from i801_block_transaction_byte_by_byte
since it's not used there any more).
-corey
This also adds timeouts when using interrupt, so a failed operation
won't hang forever.
As far as I can see, the current code already has these timeouts,
although maybe it wasn't the case when you first wrote this patch? This
was added by b3b8df9772 in November 2014.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
