The diff D43811 has been split into two parts. The first part, D44020, consists in the splitting of the original vf_i2c driver between the FDT-specific code and the I2C controller code, as well as the introduction of ACPI-specific code. The driver files are also moved to a new recommended location. This second part changes the I2C controller logic so it is more consistent with the reference manual for the controller. I used Chapter 21 from the LX2160A Reference Manual, Rev. 1, 10/2021. The code from the original vf_i2c driver did not work with the LX2160A platform. Also there were lots of delays throughout the code that did not seem necessary.
Details
Details
- Reviewers
andrew manu bz - Commits
- rG44847114bd4c: vf_i2c: update I2C controller logic
Basic testing SolidRun LX2160A CEX7 using the DK2 UEFI firmware in ACPI mode:
dmesg:
vf_i2c_acpi0: <Vybrid Family Inter-Integrated Circuit (I2C)> iomem 0x2000000-0x200ffff irq 5 on acpi0
vf_i2c_acpi0: Using existing bus frequency divider register value (0xA2).
iicbus0: <Philips I2C bus (ACPI-hinted)> on vf_i2c_acpi0
iicbus0: <unknown card> at addr 0x77
iic0: <I2C generic I/O> on iicbus0
vf_i2c_acpi1: <Vybrid Family Inter-Integrated Circuit (I2C)> iomem 0x2010000-0x201ffff irq 6 on acpi0
vf_i2c_acpi1: Bus frequency divider value appears unset, defaulting to low I2C bus speed.
iicbus1: <Philips I2C bus (ACPI-hinted)> on vf_i2c_acpi1
iic1: <I2C generic I/O> on iicbus1
Right after boot:
i2c -s -f /dev/iic0
19 1b 36 37 50 51 53 57 77
After selecting MUX's channel 1 (MUX address is 0x77, I used the code https://github.com/pldrouin/amc6821_controller to select channel 1 on LX2160A and to query the ID of the fan controller):
i2c -s -f /dev/iic0
18 6a 77
TZ0 is still working. Have not witnessed interference between the thermal zone and the I2C controller driver so far:
sysctl hw.acpi.thermal.tz0
hw.acpi.thermal.tz0._TSP: 50
hw.acpi.thermal.tz0._TC2: 1
hw.acpi.thermal.tz0._TC1: 1
hw.acpi.thermal.tz0._ACx: 80.1C -1 -1 -1 -1 -1 -1 -1 -1 -1
hw.acpi.thermal.tz0._CRT: 95.1C
hw.acpi.thermal.tz0._HOT: -1
hw.acpi.thermal.tz0._CR3: -1
hw.acpi.thermal.tz0._PSV: 55.1C
hw.acpi.thermal.tz0.thermal_flags: 0
hw.acpi.thermal.tz0.passive_cooling: 0
hw.acpi.thermal.tz0.active: 1
hw.acpi.thermal.tz0.temperature: 44.1C
Retrying bus scanning:
i2c -s -f /dev/iic0
18 6a 77
Support for some other systems in FDT mode should be tested as well
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 56313 Build 53201: arc lint + arc unit
Event Timeline
Comment Actions
WOW. Having looked at that logic probably a year ago this is good work; also good catches on the missing unlock and NOACK for the 1 byte!
I feel sorry about all the white space change requests.
| sys/dev/iicbus/controller/vybrid/vf_i2c.c | ||
|---|---|---|
| 148 | please restore; this is currently just noise in the diff. | |
| 178 | Also just a blank line change. | |
| 249 | Blank line after declarations. | |
| 254–261 | spaces around the "=" | |
| 266 | Wonder why this line changed. Spaces in it before or after? | |
| 339 | if bus [insert: is] still busy[insert .] */ | |
| 390 | is this 2 tabs or are there spaces in there? | |
| 470 | remove empty line again | |
| 478–483 | the entire new block is marked in two greens in Phabricator; are there spaces and tabs mixed? | |
| 484–485 | remove empty line | |
| 490–494 | remove space after last | |
| 491 | remove empty line | |
| 522 | remove empty line | |
Comment Actions
- Merge changes that were done to D44020 into here
- Implementing all bz's recommendations
Comment Actions
- Merging readability change from D44020.
- Fixing a bug that was introduced in Diff 134928 (a pointer was not properly declared).
Comment Actions
Sorry the previous diff had been generated using an outdated version of the D44020 diff.