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 - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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.
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?

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).

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

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

add to modules/Makefile

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.

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.

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

goldmumble wasn't it?

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".

rebase to the recent head

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.

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.

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?

  • 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

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.