lkml.org 
[lkml]   [2011]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCHv2 4/7] input/cma3000_d0x: Add CMA3000 spi support
Hello Jonathan

I have separated the read and write commands... but when I was
implementing the spi_write I found that most of the drivers just use
stack variables for spi_write. Like drivers/hwmon/ads7871.c,
drivers/gpio/max7301.c and others... Am I missing something or it is a
general "bug"?

As you say, it is better to not rewrite the cma3000 driver until
Hemanth says so, lets leave it for a future patch.




On Tue, Oct 18, 2011 at 15:50, Jonathan Cameron <jic23@cam.ac.uk> wrote:
> On 10/18/11 14:43, Ricardo Ribalda Delgado wrote:
>> 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
> It's somewhat unconventional to verify the existence of a chip like this.
> Usually you assume that if it was there once it still is unless there
> is a very good reason to think otherwise.  Worth doing an initial check
> in your spi_probe and indeed verify there against these known bits.
> No need to do it every time though.
>>
>> - 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?
> That's absolutely fine and the right way to do it. You could poke it
> into the cma3000_accl_data then use the cachline aligned magic. Its
> is tiny so I doubt anyone will mind the overhead for the i2c side of
> things.
>>
>> -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?
> I would. Though probably worth getting Hemanth to say if he minds first
> given it's his driver!
>>
>>
>> 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-10-18 17:21    [W:0.179 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site