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.
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.
-
David Hendricks -
Stefan Tauner