linux-kernel - Re: [PATCHv2 4/7] input/cma3000

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPybu_0kxqfo9Lx9V_oOXfBjOG2dnR0NLCV6=PM6cnUYmVb_iQ@mail.gmail.com>
Date:	Tue, 18 Oct 2011 15:43:28 +0200
From:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
To:	Jonathan Cameron <jic23@....ac.uk>
Cc:	dmitry.torokhov@...il.com, sameo@...ux.intel.com,
	peter.ujfalusi@...com, aghayal@...eaurora.org, david@...deman.nu,
	Shubhrajyoti@...com, saaguirre@...com, hemanthv@...com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 4/7] input/cma3000_d0x: Add CMA3000 spi support

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/