On Thu, Oct 11, 2012 at 7:44 PM, Stefan Tauner <stefan.tauner@student.tuwien.ac.at> wrote:
On Thu, 11 Oct 2012 19:16:40 -0700
David Hendricks <dhendrix@google.com> wrote:

> On Wed, Oct 3, 2012 at 3:59 PM, Carl-Daniel Hailfinger <
> c-d.hailfinger.devel.2006@gmx.net> wrote:
>
> > Not for merge... yet.
> > It works for dummy, nothing else was tested.
> > Limitations/bugs mentioned in the patch.
> >
>
> Interesting approach. However, I think 3-byte vs. 4-byte RDID commands and
> caching REMS might make patching spi_send_command rather messy.
>
> I made a patch that takes a different route by changing
> probe_spi_{rdid,rdid4,rems} functions instead. My patch can be viewed via
> Gerrit on Chromium.org @
> https://gerrit.chromium.org/gerrit/#/c/35376/2 (doesn't
> apply cleanly against upsteam currently).
>

Much better, but still wrong :) We work around a stupid probing loop
instead fixing the root cause (verbose prints will still be way too
verbose with this patch). If there are good reasons to do it this way,
then i have no problem with it, but if we just hack this into the SPI
code because it is easier to implement and come to a consensus, then
i'll make the latter hard :P

Fixing the probing loop has been on my todo list for a long time and i
will work on it as soon as the other architectural changes are merged
(status register stuff, check_trans etc). We should postpone the
discussion until then IMHO. I suggest that chromium uses David's method
till then and someone reviews my other patches soon ;) OTOH it would
not hurt to integrate this into upstream with one exception:
it would introdcue even more conflicts between open patches:

David: some of this heavily conflicts with my "Generify
probe_spi_rdid_generic() and add probe_spi_rdid_edi()." and other
patches from that set. Maybe it would be better if you leave out the
compare_id() introduction to get less conflicts later.

Haha, totally agreed :-) We really should make the probe loop smarter about caching results rather than hacking this into the chip code.

You also have a good point about conflicts in my patch. Though there will be plenty of conflicts without the compare_id() stuff anyway... I'll just revert my change when we get a proper method implemented.