Page MenuHomeFreeBSD

Add ACPI attachment for the Vybrid VF610 I2C controller
Changes PlannedPublic

Authored by val_packett.cool on May 19 2020, 2:33 PM.
Referenced Files
Unknown Object (File)
Mon, Apr 8, 9:28 AM
Unknown Object (File)
Feb 20 2024, 9:11 AM
Unknown Object (File)
Jan 16 2024, 11:08 PM
Unknown Object (File)
Dec 29 2023, 9:59 AM
Unknown Object (File)
Dec 29 2023, 9:55 AM
Unknown Object (File)
Dec 26 2023, 8:29 AM
Unknown Object (File)
Dec 20 2023, 7:07 AM
Unknown Object (File)
Dec 18 2023, 3:31 PM

Details

Reviewers
mw
manu
dgr_semihalf.com
andrew
Group Reviewers
arm64
Summary

This expands on D24361, splitting FDT into a separate file and adding ACPI, to support the SolidRun LX2160A CEX7 module (e.g. on the SolidRun HoneyComb LX2K workstation board).

Ignore the files.arm64, I can fix that after D24361 and stuff lands (note: I don't like the SOC_NXP_LS1046A option, that's overly specific, this kind of hardware is common to the whole layerscape family)

Test Plan

Tested on: SolidRun LX2160A CEX7, by Dan Kotowski on freebsd-arm@

i2c0: <Vybrid Family Inter-Integrated Circuit (I2C)> iomem 0x2000000-0x200ffff irq 7 on acpi0
iicbus0: <Philips I2C bus (ACPI-hinted)> on i2c0
iicbus0: <unknown card> at addr 0x77
iic0: <I2C generic I/O> on iicbus0
i2c1: <Vybrid Family Inter-Integrated Circuit (I2C)> iomem 0x2040000-0x204ffff irq 8 on acpi0
iicbus1: <Philips I2C bus (ACPI-hinted)> on i2c1
iic1: <I2C generic I/O> on iicbus1

seems to detect all numbers as present though o_0 that's probably not good

Scanning I2C devices on /dev/iic0: 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e

Maybe setting the freq would help..

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

val_packett.cool edited the summary of this revision. (Show Details)
val_packett.cool edited the test plan for this revision. (Show Details)

Looks like this needs to wait until there's some way to get the clock without devicetree (I guess Linux runs with fdt+acpi both to make it work..)

In D24917#550057, @greg_unrelenting.technology wrote:

Looks like this needs to wait until there's some way to get the clock without devicetree (I guess Linux runs with fdt+acpi both to make it work..)

That don't make sense to run in fdt+acpi, I don't even think that it is possible in Linux.

In D24917#550060, @manu wrote:
In D24917#550057, @greg_unrelenting.technology wrote:

Looks like this needs to wait until there's some way to get the clock without devicetree (I guess Linux runs with fdt+acpi both to make it work..)

That don't make sense to run in fdt+acpi, I don't even think that it is possible in Linux.

Clock subsystem is out of OS control when running with Linux + ACPI and IMO this should be true for the FreeBSD as well. In such case we rely on firmware to manage the power/clocking stuff.

In D24917#550242, @mw wrote:
In D24917#550060, @manu wrote:
In D24917#550057, @greg_unrelenting.technology wrote:

Looks like this needs to wait until there's some way to get the clock without devicetree (I guess Linux runs with fdt+acpi both to make it work..)

That don't make sense to run in fdt+acpi, I don't even think that it is possible in Linux.

Clock subsystem is out of OS control when running with Linux + ACPI and IMO this should be true for the FreeBSD as well. In such case we rely on firmware to manage the power/clocking stuff.

Well then how does the driver for NXP0001

https://github.com/torvalds/linux/blob/bef7b2a7be28638770972ab2709adf11d601c11a/drivers/i2c/busses/i2c-imx.c#L258

unconditionally require a platform clock in probe?

https://github.com/torvalds/linux/blob/bef7b2a7be28638770972ab2709adf11d601c11a/drivers/i2c/busses/i2c-imx.c#L1160-L1166

Linux's acpi_apd integration seems to be really deep, I guess these devices kind of become proper OFW platform members

In D24917#550267, @greg_unrelenting.technology wrote:
In D24917#550242, @mw wrote:
In D24917#550060, @manu wrote:
In D24917#550057, @greg_unrelenting.technology wrote:

Looks like this needs to wait until there's some way to get the clock without devicetree (I guess Linux runs with fdt+acpi both to make it work..)

That don't make sense to run in fdt+acpi, I don't even think that it is possible in Linux.

Clock subsystem is out of OS control when running with Linux + ACPI and IMO this should be true for the FreeBSD as well. In such case we rely on firmware to manage the power/clocking stuff.

Well then how does the driver for NXP0001

https://github.com/torvalds/linux/blob/bef7b2a7be28638770972ab2709adf11d601c11a/drivers/i2c/busses/i2c-imx.c#L258

unconditionally require a platform clock in probe?

https://github.com/torvalds/linux/blob/bef7b2a7be28638770972ab2709adf11d601c11a/drivers/i2c/busses/i2c-imx.c#L1160-L1166

Linux's acpi_apd integration seems to be really deep, I guess these devices kind of become proper OFW platform members

drivers/acpi/acpi_apd.c gives an option to mock a real clock with a fixed-rate one - acpi_apd_setup() routine registers such fixed clock in the controllers platform data. This way, the clk_get can return it without error, avoiding the OFW path (the section under if (dev && dev->of_node) { is omitted.

As you can see, this way of adding fixed clock is used for SPI/I2C/UART controllers, where you need to calculate the output rate. As mentioned before, the actual control over enabling the clocks in HW is done in firmware.

The method used in Linux is IMO awful - a central place with platform devices description only for specifying a clock rate. I know SoCs, which use same controller in various variants, but feed them with the different frequency. Therefore I think a proper way would be either add 'clock-frequency' to the _DSD package of the controller in DSDT or hardcode a frequency in the driver when working with ACPI - this is what the Linux effectively does.

Therefore I think a proper way would be either add 'clock-frequency' to the _DSD package of the controller in DSDT

It does have that:

Name (_DSD, Package () {
  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
  Package () {
     Package () {"clock-frequency", DEFAULT_PLAT_FREQ}, //This is device specific data, Need to see how to pass clk stuff
  }
})

So that's cool I guess, I'll just need to finally work on DSD access.

What confused me the most was i2c_get_div_val which does calculations with both sc->freq, which *is* the i2c device's clock-frequency, AND a frequency of a clock device! That made me think the frequency from the clock is something very dynamic.

https://github.com/freebsd/freebsd/commit/36362eb0a0a1862a0898a6e17844a54f3f28f114 looks like extra code in the main driver itself was required for this hardware to work too, and we now have that

FYI: https://uefi.org/specs/ACPI/6.5/19_ASL_Reference.html#clock-input-resource-descriptor-macro reports:

19.6.154. ClockInput (Clock Input Resource Descriptor Macro)

Looks Like the ACPI 6.5 was released on 2022-Aug-29, along with UEFI 2.10.

Looks like this needs to wait until there's some way to get the clock without devicetree (I guess Linux runs with fdt+acpi both to make it work..)

ACPI 6.5 now has ClockInput Resource Descriptors that 6.4 did not have: 6.4.3.14 Clock Input Resource Descriptor

Might this be the kind of thing needed? (I do not know the acpica status for any version, and FreeBSD main [so: 15] is a couple of acpica releases behind at this point.) EDK2 and FreeBSD might both need changes to involve such descriptors, even once FreeBSD is using a acpica that allows for such descriptors.

I wrote a FDT/ACPI split driver for the LX2160A I2C controller based on the work started by Val, and the LX2160A I2C controller documentation. It deviates significantly from the Vybrid driver as the way the original driver was handling the controller's registers was not quite compatible with the LX2160A I2C controller. Should I start a new diff with my driver and link to it from here? It seems to work well so far based on the (limited) tests I made. I can scan the bus and get the correct output. I am able to query the MUX's register and the fan controller's ID.

Diff here: https://reviews.freebsd.org/D43746