Re: [PATCHv2 4/7] input/cma3000_d0x: Add CMA3000 spi support

From: Ricardo Ribalda Delgado
Date: Tue Oct 18 2011 - 09:44:10 EST


Hello Jonathan

First of all, thanks for your messages :).

> To make my point about these functions being more complex than needed
> in more detail....
>
> If this were two functions and you drop the zero and 1 mask
> (which I'm not convinced make any sense. I've also killed the message.
> We both agree it is the wrong way to go, so post a patch fixing the i2c
> interface as well.

Of course your functions are much more simpler and beautiful than the
fat one I wrote, no doubt about it :). Just three comments

- Checking the one mask and the zero mask is the only way we have to
know if the chip is still there, The absense of that reply should
trigger an IO error or at least a retry. As you point out, the
zero/one mask is only violated on startup. I just wanted to make it
more risk free, but if you believe it is more clear that way, lets
remove it

- I am not very fun of kmallocing data per write, specially when it is
part of the irq handler, and you expect this to be low latency. What
about allocating a buffer on init time, and use it with a mutex?

-I dont like the push error message to the bottom, but that will mean
a rewrite of the cma3000 driver, shall I go for it?


Thanks again, and I will post the new version when you reply this :)


--
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/