On Tue, Dec 28, 2010 at 4:13 AM, Frantisek Rysanek <Frantisek.Rysanek@post.cz> wrote:
Dear maintainers of the superiotool,

I have the following "feature request" (maybe not a bug report):
I have this idea that superiotool should be able to detect more than
one SuperIO chip in the system. Such detection of multiple chips may
work in some cases, but may fail in others.

The primary cause seems to be that
 winbond.c :: probe_idregs_winbond()
and
  ite.c :: probe_idregs_ite()
call
  if (chip_found) return;   // this is a global variable!
after every "init sequence" tried.

Therefore, after the first chip detected (of any kind),
only the first init sequence is ever tried, in any subsequent calls
to the aforementioned probe_* functions.

My immediate hack was to comment out all occurrences of
  if (chip_found) return;
in those probe functions.
Makes me wonder if this has some adverse effects, such as repeatedly
scanning some EFER bases / init sequences by probes belonging to
different chip families.
And my conclusion for the moment is that this repeated probing should
be harmless, and that the current behavior (stop after the first init
sequence tried) is flawed anyway :-)

Heh, I hacked that "if (chip_found) return;" stuff into SuperI/O tool a few months ago (http://www.coreboot.org/pipermail/coreboot/2010-August/059607.html).

The reason was to avoid duplicate listings. In my specific case, I was working with an ITE chip that does not need an entry/exit sequence. So when probe_regs_ite() was called, it would find my chip 5 times and dump its registers 5 times. Sorry if it's causing you headaches now :-/

I think long-term we should make superiotool more intelligent with regard to duplicate entries. For example, if a chip has been found at 0x2e, then superiotool should stop probing devices on port 0x2e and continue probing devices on ports 0x4e, 0x3f0, etc (as listed in superioto_ports_table[]). I think we could accomplish this easily:
- Add a return code to each chip's probe_idregs_* function.
- Update the loop in main() which iterates thru superio_ports_table[]. If a probe_idregs_* function returns a specific code to indicate a chip has been found, then record the corresponding superio port entry and avoid further calls to superio_ports_table[i].probe_idregs() on that port.
 

My motivation is simple: right now I'm working with an embedded PC
that has two SuperIO chips on the inside: W83627 + W83977.
But I've seen other models in the past as well.

Your SuperIOtool is quite useful for a quick overview of the SuperIO
chips in the system, including a thorough dump of their registers.
In the past I myself have written and reused some simple snippet of
code / a tiny library (in DOS) to help me debug/tweak SuperIO stuff,
but I never got as far as making it truly universal and extensible.
As for SuperIOtool, I could fancy a writing capability, and maybe an
external configuration file, potentially allowing for extensions
without coding in C, maybe human-readable interpretations /
annotations of the register dump etc - but I admit that's too much
work to be worth the bother, certainly for me at the moment :-)

Keep up the excellent work that you're doing...

Frank Rysanek


--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot



--
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.