Winbond chips - design questions

Winbond chips - design questions

[Mark M. Hoffman]

* Mark M. Hoffman <mhoffman@lightlink.com> [2003-10-21 21:51:13 -0400]:
> OK, I committed a partial fix - go ahead and try it.  I already see that
> sensors doesn't report fan3 (which corresponds to "Power Fan" in P4C800
> BIOS.)  I will start another thread on the mailing list about that...

OK, new subject anyway...

What is the benefit of having all of the Winbond drivers in two files?

And related - what is the benefit of recycling feature tables etc. between
all the Winbond types in lib/chips.c and prog/sensors/chips.c?

I can tell you the downside, for sure: I'm afraid to touch any of it for
fear of breaking one of the other 5 chips that I *don't* have and *can't*
test.  Sure it's free software and I don't have any obligation... but right
now those particular bits are nigh unmaintainable.

I guess I'm asking for permission (and help!) in refactoring the Winbond
drivers.  The NatSemi (lm??) drivers are closer to where I think we
should go: two (or at most, three) related chips per file... and only
if they are trivially different or one has a subset of features of the
other, etc.

Also, what (kinds of) changes in libsensors will cause an ABI change?  Is
it absolutely limited to the contents of lib/sensors.h?

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

Winbond chips - design questions

[Jean Delvare]

> OK, new subject anyway...
> 
> What is the benefit of having all of the Winbond drivers in two files?

Benefit is obvious. It is because, hmmm... well... well you know, it's
obviously obvious ;)

> And related - what is the benefit of recycling feature tables etc.
> between all the Winbond types in lib/chips.c and prog/sensors/chips.c?

Almost as obvious as above ;)

> I can tell you the downside, for sure: I'm afraid to touch any of it
> for fear of breaking one of the other 5 chips that I *don't* have and
> *can't* test.  Sure it's free software and I don't have any
> obligation... but right now those particular bits are nigh
> unmaintainable.

I definitely agree with you on that point. Likewise, the adm1021 driver
has become a complete mess.

> I guess I'm asking for permission (and help!) in refactoring the
> Winbond drivers.  The NatSemi (lm??) drivers are closer to where I
> think we should go: two (or at most, three) related chips per file...
> and only if they are trivially different or one has a subset of
> features of the other, etc.

That's what I'd call a good objective. That's how I intend to make
things in drivers I wrote or maintain (lm83, lm90, adm1025).

> Also, what (kinds of) changes in libsensors will cause an ABI change? 
> Is it absolutely limited to the contents of lib/sensors.h?

Good question. Someone here, can't remember who, once said we would have
to change the library's version with each release. So I guess that the
ABI is likely to change that often. I don't quite understand what
exactly cause incompatibilities, nor what our numbering policy should
be. If someone is willing to explain that in details, that would be
welcome.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

Winbond chips - design questions

[Mark Studebaker]

It's my doing so I can answer your questions.

There's not much good reason for the monster w83781d driver.
I'm not defending it, just explaining how we got here.

Generally I would start supporting a new chip by
simply recognizing the ID and treating it like an old chip.
Later (maybe much later) when a datasheet was available, 
when the unique parts of the new chip were known,
the driver was enhanced with more if statements
to support the new chip better.

Recently, I stopped the madness and started a new driver, w83627hf,
specifically to support the super i/o chips.
The old driver works for the super i/o chips only
when their i2c address has been set by the bios or
when their isa address is 0x290. It does not include super i/o
access or detection mechanisms.

The new driver is isa-only and includes super i/o
detection, so that the isa address is always discovered.
This makes the new driver much better at finding a chip.

You could make the argument that I should have removed
627hf/697hf support from the w83781d driver at the same time.
But I didn't want to disrupt people that were using that
driver (see below). And remember the new driver is isa-only.
So if you want i2c access for whatever reason you have to
use the old driver.

Why recycle stuff in libsensors? just laziness.
It's a pain to retype all that stuff. Plus if the
#defines are different then you need a whole different
print function in prog/sensors/chips.c.
For big chips like the winbonds that's a _lot_ of extra work
to add support.

I would claim the w83627hf driver is in good shape but I'm
open to suggestions.

If you do want to tackle a w83781d rewrite it's fine w/ me.
You should decide whether you want to do it in 2.4 or 2.6.
But a couple of warnings.
You should have a good sample of boards with these chips to test with.
And you should be careful. A _huge_ number of people have chips
supported by the w83781d driver. When a release takes a step
backwards or changes something in that driver we get a ton of mail.
It's not fun.
Not to mention the whole undocumented as99127f chip which
is, historically, probably our #1 chip for complaints.



mds




"Mark M. Hoffman" wrote:
> 
> * Mark M. Hoffman <mhoffman@lightlink.com> [2003-10-21 21:51:13 -0400]:
> > OK, I committed a partial fix - go ahead and try it.  I already see that
> > sensors doesn't report fan3 (which corresponds to "Power Fan" in P4C800
> > BIOS.)  I will start another thread on the mailing list about that...
> 
> OK, new subject anyway...
> 
> What is the benefit of having all of the Winbond drivers in two files?
> 
> And related - what is the benefit of recycling feature tables etc. between
> all the Winbond types in lib/chips.c and prog/sensors/chips.c?
> 
> I can tell you the downside, for sure: I'm afraid to touch any of it for
> fear of breaking one of the other 5 chips that I *don't* have and *can't*
> test.  Sure it's free software and I don't have any obligation... but right
> now those particular bits are nigh unmaintainable.
> 
> I guess I'm asking for permission (and help!) in refactoring the Winbond
> drivers.  The NatSemi (lm??) drivers are closer to where I think we
> should go: two (or at most, three) related chips per file... and only
> if they are trivially different or one has a subset of features of the
> other, etc.
> 
> Also, what (kinds of) changes in libsensors will cause an ABI change?  Is
> it absolutely limited to the contents of lib/sensors.h?
> 
> Regards,
> 
> --
> Mark M. Hoffman
> mhoffman@lightlink.com

Winbond chips - design questions

[Mark M. Hoffman]

* Mark D. Studebaker <mds@paradyne.com> [2003-10-22 20:32:55 -0400]:
<snip>
> Recently, I stopped the madness and started a new driver, w83627hf,
> specifically to support the super i/o chips.
> The old driver works for the super i/o chips only
> when their i2c address has been set by the bios or
> when their isa address is 0x290. It does not include super i/o
> access or detection mechanisms.
> 
> The new driver is isa-only and includes super i/o
> detection, so that the isa address is always discovered.
> This makes the new driver much better at finding a chip.

Is this just ISA PnP?  I have no experience with that.

<snip>
> Why recycle stuff in libsensors? just laziness.
> It's a pain to retype all that stuff. Plus if the
> #defines are different then you need a whole different
> print function in prog/sensors/chips.c.
> For big chips like the winbonds that's a _lot_ of extra work
> to add support.

Yes I've noticed - while working on an independent asb100 driver.

I'm not as familiar with the userland side as the kernel side.
Some of that drudgery could be tamed by macros.  But that's the
least of it really...

Why are those enormous tables present in libsensors at all?
And then again what is the point of libsensors if the
sensors program needs special code for each driver?  Yuck.
I guess I should go read the source of gkrellm and see what
they're doing.

> I would claim the w83627hf driver is in good shape but I'm
> open to suggestions.

As I mentioned, there's something broken about fan3.  I'm going
to look at that first.

> If you do want to tackle a w83781d rewrite it's fine w/ me.
> You should decide whether you want to do it in 2.4 or 2.6.
> But a couple of warnings.
> You should have a good sample of boards with these chips to test with.

I have a w83781d and a w83627thf.  A friend of mine apparently has
a closet full of old boards that he doesn't want anymore... so I
could get lucky and find more.

> And you should be careful. A _huge_ number of people have chips
> supported by the w83781d driver. When a release takes a step
> backwards or changes something in that driver we get a ton of mail.
> It's not fun.
> Not to mention the whole undocumented as99127f chip which
> is, historically, probably our #1 chip for complaints.

If the only difference is that you load w83wxyz instead of w83781d
after an upgrade - I don't see that as a problem.  We can warn
people against that on the website.  At least their sensors.conf
won't need to change.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

Winbond chips - design questions

[Mark Studebaker]

"Mark M. Hoffman" wrote:
> 
> * Mark D. Studebaker <mds@paradyne.com> [2003-10-22 20:32:55 -0400]:
> <snip>
> >
> > The new driver is isa-only and includes super i/o
> > detection, so that the isa address is always discovered.
> > This makes the new driver much better at finding a chip.
> 
> Is this just ISA PnP?  I have no experience with that.
> 

I don't know PnP so I don't know.
You can look in the driver or in sensors-detect to see
what super i/o looks like. If there's a real name
for it I don't know what it is. I just followed the
super i/o datasheets then realized all the chips were similar.

> <snip>
> 
> 
> Why are those enormous tables present in libsensors at all?
> And then again what is the point of libsensors if the
> sensors program needs special code for each driver?  Yuck.
> I guess I should go read the source of gkrellm and see what
> they're doing.
> 
>

Agreed that libsensors is problematic.
One reason why I've pushed for and documented firm /proc standards
as an alternative.