Page MenuHomeFreeBSD

add superio driver
ClosedPublic

Authored by avg on Oct 7 2016, 10:15 AM.

Details

Summary

The driver is designed to provide an easier access to functions
that SuperIO chips provide and which are not advertised in other
ways. Examples of functions that the superio driver is not going
to support: keyboard, mouse, serial and parllel ports, floppy
controller. Examples of functions for which the superio driver
is useful: hardware monitoring, watchdog timer, GPIO.

The main purpose of the driver is to consolidate discovery of
the supported hardware, its functions and resources.
For example, most of what wbwd(4) does in its probing code should
be moved to this driver.

*This is not a commit material yet.*
Rather I would like to discuss what and how this driver should do
using some concrete code as a base for the discussion.

There are some choices that I have not made yet.
For example, I see at least three possibilites for how this driver
should interact with drivers for specific functions:

  1. This driver works only as an information provider. Other drivers use it to discover the logical functions and resources, but they actually attach to other buses like isa.
  2. This driver works as a bus that has a global knowledge about drivers for the SuperIO functions. So, superio creates named child devices to attach predefined drivers to them.
  3. This driver works as a bus and creates wildcard devices for the known functions. Other drivers can bid for those devices.

In the last two cases the driver could also allow to add hinted and
self-identifying devices.

Also, I am not sure if it's okay to provide the superio interface
in the form of directly callable functions or if an interface should
be defined.
The simple interface seems to work, but it will blow up if it's used
on a wrong device by mistake.

superio currently works only with a number of ITE SuperIO chips.
To do:

  • support more chips, including all known by wbwd
  • make wbwd work via superio
  • add itwd driver for watchdogs in ITE devices
  • consider which functions should become methods
  • consider setting up resources (i/o ranges) to enumerated devices
  • maybe add support for self-identifying drivers and hinted devices

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

avg updated this revision to Diff 21138.Oct 7 2016, 10:15 AM
avg retitled this revision from to add superio driver.
avg updated this object.
avg edited the test plan for this revision. (Show Details)
avg added reviewers: imp, jhb.
imp added inline comments.Oct 8 2016, 4:35 AM
sys/dev/superio/superio.c
54 ↗(On Diff #21138)

I know that sio stands for super I/O here, but sio was the serial driver for years and there's an immediate reaction to it :)

254–255 ↗(On Diff #21138)

Other drivers that do identify will put extra code to prevent that. Though looking at the probe routine it looks like this might be difficult...

315 ↗(On Diff #21138)

How do we even get here? We continue a few lines up if vendor != ITE, so how can it be NUVOTON here?

avg added inline comments.Oct 10 2016, 2:05 PM
sys/dev/superio/superio.c
54 ↗(On Diff #21138)

Yes, I know. I used the full name in the interface, but decided that using the shorthand is acceptable for the implementation.
Not a problem to do s/sio/superio/ if that would be preferred.

254–255 ↗(On Diff #21138)

I see that wbwd driver has to solve a similar problem. I'm not sure if I like its solution, but it certainly works.
Essentially, the low level I/O methods accept both an IO resource and a numeric port. So they can be used both before a device is created and after resources are set up.

Maybe the following could work?
The device_probe method is reduced to always returning success. Instead the current probe code is called directly from the identify method with a child device. If probing fails then the device is removed.

315 ↗(On Diff #21138)

It's a place holder for when the Nuvoton support is added. Just so that I don't forget that the device ID is read differently (some register but byte vs word).

avg updated this revision to Diff 21296.EditedOct 12 2016, 7:27 AM

superio: avoid creating devices for hadrware that is not there
Perform hardware probing in the identify method.

avg marked 4 inline comments as done.Oct 12 2016, 7:28 AM
avg updated this revision to Diff 21346.Oct 13 2016, 7:38 AM

Allow to be compiled into kernel.
Add to NOTES as well.

avg updated this revision to Diff 21412.Oct 15 2016, 8:40 AM

rebase

avg updated this revision to Diff 21440.Oct 17 2016, 8:13 AM

add to modules/Makefile

avg updated this object.Oct 17 2016, 8:13 AM
avg updated this revision to Diff 21443.Oct 17 2016, 9:17 AM
avg updated this object.

Add suport for Nuvoton / Winbond SuperIO-s based on wbwd driver.
Note that not all of the chips have a HWM component, but a HWM
child is currently added for all.

emaste added a subscriber: emaste.Oct 19 2016, 4:20 PM
op added a subscriber: op.Nov 4 2016, 7:25 PM
op added a comment.Nov 4 2016, 8:50 PM

Seems like it successfully attaches:

root@h12 ~# sysctl dev.superio.
dev.superio.0.%parent: isa0
dev.superio.0.%pnpinfo:
dev.superio.0.%location:
dev.superio.0.%driver: superio
dev.superio.0.%desc: ITE IT8728 SuperIO (revision 0x01)
dev.superio.%parent:

This is a Gigabyte H87N-Wifi mobo with ITE IT8728 chip.

julian added a subscriber: julian.Nov 24 2016, 6:02 PM

I vaguely remember another superio driver for another model/maker super io chip

goldmumble wasn't it?

avg added a comment.Nov 25 2016, 9:09 AM

I vaguely remember another superio driver for another model/maker super io chip
goldmumble wasn't it?

Sorry, I have no clue. And google doesn't turn up anything useful on "goldmumble".

avg updated this revision to Diff 58352.Jun 7 2019, 8:06 AM

rebase to the recent head

jhb added a comment.Jun 27 2019, 7:01 PM

So I looked this over and I think the general approach is ok. I would probably prefer either 2) or 3) as I think that is simpler than 1). acpi_perf for 1) ends up being problematic. My only question about 3) is what kind of ivars would you provide that other drivers would bid on? If there aren't good ones, then named drivers ala 2) is probably simpler to work with. If the layout is fixed for each chip you could perhaps have an ivar that is an enum of functions (GPIO, watchdog, etc.) and use that for 3) instead of names I guess. If there is some kind of decent ivar that makes sense for differentiating devices, then I'd be fine with 3) over 2) I think.

avg added a comment.Jun 27 2019, 8:11 PM
In D8175#449725, @jhb wrote:

So I looked this over and I think the general approach is ok. I would probably prefer either 2) or 3) as I think that is simpler than 1). acpi_perf for 1) ends up being problematic. My only question about 3) is what kind of ivars would you provide that other drivers would bid on? If there aren't good ones, then named drivers ala 2) is probably simpler to work with. If the layout is fixed for each chip you could perhaps have an ivar that is an enum of functions (GPIO, watchdog, etc.) and use that for 3) instead of names I guess. If there is some kind of decent ivar that makes sense for differentiating devices, then I'd be fine with 3) over 2) I think.

John, I actually went for 3 meanwhile. The bus provides two identity ivars for each child it creates: logical device number and a function assigned to that device. In my, admittedly limited, experience that information along with Super I/O chip ID is sufficient for the kind of drivers I would expect to use this bus: watchdogs, gpio controllers, hardware / environment monitoring functions.

jhb added a comment.Jun 27 2019, 8:52 PM

Ok, sounds good to me. I think the 'type=' might more correctly be part of the 'pnpinfo_str' method instead of 'location_str' FWIW as presumably the type is the ivar you are probing on in child drivers?

avg updated this revision to Diff 59266.Jul 1 2019, 4:08 PM

rebase

avg updated this revision to Diff 59267.Jul 1 2019, 4:20 PM
  • add "gpio" child type
  • add Nuvoton devices that are supported by nctgpio driver
  • change how Nuvoton device IDs are matched as for some higher bits of revision ID register are actually a part of a device ID
  • account for the fact that Nuvoton SuperIOs use multiple bits in the Device Enable register
  • change how known devices are defined
  • move child's "type" from the location string to the pnp string, suggested by jhb
avg updated this revision to Diff 59268.Jul 1 2019, 4:24 PM

clean up

avg updated this revision to Diff 59269.Jul 1 2019, 4:26 PM

oops, forgot to update the header file

This revision was not accepted when it landed; it landed in state Needs Review.Jul 1 2019, 5:05 PM
Closed by commit rS349580: add superio driver (authored by avg). · Explain Why
This revision was automatically updated to reflect the committed changes.