Page MenuHomeFreeBSD

Add support for Nuvoton NCT6122D/NCT6126D.
Needs ReviewPublic

Authored by Matthew.Nygard.Dodd_gmail.com on Nov 14 2024, 2:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 3, 2:49 AM
Unknown Object (File)
Mon, Dec 23, 3:07 AM
Unknown Object (File)
Sat, Dec 14, 8:30 AM
Unknown Object (File)
Dec 11 2024, 8:40 AM
Unknown Object (File)
Dec 10 2024, 3:45 AM
Unknown Object (File)
Dec 9 2024, 12:31 PM
Unknown Object (File)
Nov 21 2024, 9:15 PM
Unknown Object (File)
Nov 18 2024, 9:28 AM

Details

Reviewers
imp
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Add support for Nuvoton NCT6122D/NCT6126D - superio(9), nctgpio(9), ncthwm(9)

  • GPIO for LDN 7 & 9.
  • HWM for LDN 11
  • Provide LDN based matching for GPIO devices.
  • Provide gpio(9) hint flag (NCT_NO_ENABLE) for skipping GPIO enable, to avoid altering BIOS setting.
  • Allocate GPIO ioports as shareable.
  • Constrain mask based matching for existing NCT6112D/NCT6114D/NCT6116D.
Test Plan

Tested on Asus N97T-IM-A:

(dmesg output)
superio0: <Nuvoton NCT6122D/NCT6126D> at port 0x2e-0x2f on isa0
gpio0: <GPIO on Nuvoton NCT6122D/NCT6126D> at GPIO ldn 0x07 on superio0
gpio0: skipping enable.
gpiobus0: <GPIO bus> on gpio0
gpioc0: <GPIO controller> on gpio0
gpio1: <GPIO on Nuvoton NCT6122D/NCT6126D> at GPIO ldn 0x09 on superio0
gpio1: skipping enable.
gpiobus1: <GPIO bus> on gpio1
gpioc1: <GPIO controller> on gpio1
ncthwm0: <HWM on Nuvoton NCT6122D/NCT6126D> at HWM ldn 0x0b on superio0

(sysctl)
dev.ncthwm.0.AUXFAN2: 0
dev.ncthwm.0.AUXFAN1: 0
dev.ncthwm.0.AUXFAN0: 0
dev.ncthwm.0.CPUFAN: 0
dev.ncthwm.0.SYSFAN: 796

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Owners added a reviewer: Restricted Owners Package.Nov 14 2024, 2:55 PM

If you have added this from a diff, could you please re-do the diff with -U 9999 option?
That's to have a lot more context lines which would be useful when reviewing.

In D47571#1084984, @avg wrote:

If you have added this from a diff, could you please re-do the diff with -U 9999 option?
That's to have a lot more context lines which would be useful when reviewing.

Done.

In D47571#1084984, @avg wrote:

If you have added this from a diff, could you please re-do the diff with -U 9999 option?
That's to have a lot more context lines which would be useful when reviewing.

Done.

Phabricator still tells me "Context not available".

This seems good to my eye...
Do you need someone to commit it?

This revision is now accepted and ready to land.Nov 16 2024, 6:10 AM
In D47571#1086063, @imp wrote:

This seems good to my eye...
Do you need someone to commit it?

My commit bit has been inactive for years so yea, probably. :)

sys/dev/superio/superio.c
480

If my understanding of the datasheet is correct, page 313 state the chip IDs are 0xd2a3 for NCT6122D and 0xd283 for NCT6126D. So I would expect a mask here, so I wonder what I miss?

sys/dev/superio/superio.c
480

It's more likely that I missed something. I did spend a bunch of time looking for Chip ID data and what I found wasn't clear.

The version of the datasheet I've got is 21 Feb 2020, revision 0.9, which documents the default value for CR 0x20/0x21 as 0xd2/0x81, which clearly doesn't match the hardware in hand.

Any chance I could get a copy of your datasheet? :)

sys/dev/superio/superio.c
480

Sent by MP.

Will update patch based on docs provided by S.R.

Directly match NCT6112D/NCT6114D/NCT6116D/NCT6122D/NCT6126D by Chip ID, per datasheets(ish).

This revision now requires review to proceed.Nov 25 2024, 11:05 PM
sys/dev/superio/superio.c
480–499

I have a NCT6112D at hand that advertise chip ID 0xd282.

I have no way to reliably know the chip ID for NCT6114D and NCT6116D. From their datasheet, page 294, I can only guess one of them is 0xd281. And we also know that 0xd283 is NCT6126D according to its datasheet, page 313. To sum up:

NameChip ID
NCT6112D0xd282
NCT6114D0xd28?
NCT6116D0xd28?
NCT6122D0xd2a3
NCT6126D0xd283

Note that in the past Nuvoton already arranged to create some chip ID clashes. That's why .extid was introduced :/

sys/dev/superio/superio.c
480–499

The NCT6126D I've got here is 0xd284.

sys/dev/superio/superio.c
480–499

Well, who would ever trust a datasheet anyway? ;)

sys/dev/superio/superio.c
480–499

So how about a specific match for 0xd282, then a masked match for 0xd280/0x0003 to sweep up what that missed and then the specific match for the NCT6122D/NCT6126D?

We could put the specific match entries first and have the masked match with the original mask of 0x00ff but that feels clunky.

sys/dev/superio/superio.c
480–499

So how about a specific match for 0xd282, then a masked match for 0xd280/0x0003 to sweep up what that missed and then the specific match for the NCT6122D/NCT6126D?

Looks reasonable to me.

sys/dev/nctgpio/nctgpio.c
756–758

I don't understand the purpose of LDN-based matching. Is there a problem with having .ngroups = 11 ?

sys/dev/ncthwm/ncthwm.c
128–129

I find kind of weird not to adopt the same mask semantic as the one found in superio_detect:

			if ((superio_table[i].devid & ~mask) != (devid & ~mask))
				continue;

Update hwm/gpio mask matching to use existing superio convention.

sys/dev/nctgpio/nctgpio.c
756–758

#define NCT_MAX_GROUP 9

I'd kinda like to turn nct_device groups into a pointer since we already have ngroups. This would permit arbitrary sized nct_gpio_group arrays without wasted space.

sys/dev/nctgpio/nctgpio.c
756–758

Wouldn't it imply that nct_device would become dynamically allocated? Or do you think using flex array would do the thrick?

Hi @Matthew.Nygard.Dodd_gmail.com and happy new year :)
I got some more data about the chip IDs:

NameChip ID
NCT6112D0xd282
NCT6114D0xd282
NCT6116D0xd282
NCT6122D0xd2A4
NCT6126D0xd284

As you can see the datasheets are incorrects and the NCT611x share the same chip ID which means we probably need .prefer for them.