[flashrom] [PATCH] Cache SPI RDID response

David Hendricks david.hendricks at gmail.com
Fri Oct 12 05:43:48 CEST 2012


On Thu, Oct 11, 2012 at 7:44 PM, Stefan Tauner <
stefan.tauner at student.tuwien.ac.at> wrote:

> On Thu, 11 Oct 2012 19:16:40 -0700
> David Hendricks <dhendrix at google.com> wrote:
>
> > On Wed, Oct 3, 2012 at 3:59 PM, Carl-Daniel Hailfinger <
> > c-d.hailfinger.devel.2006 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20121011/3f12c433/attachment.html>


More information about the flashrom mailing list