Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

[Andrew Cooks <acooks@rationali.st>]

On 8/20/19 4:30 AM, Enrico Weigelt, metux IT consult wrote:
> On 11.08.19 04:52, Andrew Cooks wrote:
>
> Hi,
>
>> My initial work is based on a board that is similar to the APU2, but has additional peripherals connected to the smbus, including a NCT7491 thermal monitor/fan controller and PCA6524 GPIO controller. These are simply peripherals on a board variant, not 'platform' devices, so I didn't want to follow the platform driver approach that the APU2 GPIO driver uses.
> Why are they not platform devices ?
>
> IMHO, a platform device is something not on a bus that can do the
> probing/enumeration (like eg. pci or usb does).
>
> Can these devices be probed directly by the bus they're connected to,
> or do you have a different definition of the term "platform device" ?

ACPI was created to provide the PNP mechanism for buses that do not support enumeration, like SMBus. SMBus is similar to I2C, and can be routed to expansion cards, like Ethernet NICs. A system with expansion card Foo is not a different platform from a system with expansion card Bar, and you wouldn't necessarily want to support both Foo and Bar in the same platform driver.

>
> :O
>
>> SMBus (and I2C) peripherals can generally not be enumerated without some firmware support. It is possible to probe for specific devices on the bus (eg sensors-detect) but in general it is not feasible to let every supported device driver probe the bus for its device. ACPI and Devicetree provides the kernel with metadata for the device: type, address, calibrated set points for temperature, etc.
> Yes, I know. My question was whether these particular devices - if
> they're in the SoC itself - do need some extra support in BIOS
> (acpi entries, etc), that's not already in aegesa. Or in other words:
> do I need to patch my BIOS ?
>
> I was assuming these devices are in the SoC itself - correct me if
> I'm wrong.

This patch is about adding a mechanism to describe peripherals that are not in the SoC, but attached to the SMBus. I also provided examples of the kinds of devices that need this support (eg. NCT7491).

>
>> Since the peripherals are not standard platform devices, they are not described by the ACPI tables provided by Coreboot or AMD, but it's not too difficult to create supplementary device description tables (ACPI) for non-standard devices. These can be added to coreboot, supplied to qemu as additional firmware files (see -acpitable arg), or built into the kernel (see https://www.kernel.org/doc/Documentation/acpi/ssdt-overlays.txt)
> Oh, I didn't know about SSDT overlays yet. That's interesting.
>
> Is it also possible to describe things like leds-gpio or gpio-keyboard ?
> Could be a nice idea to trim down my apu2 driver to not much more than
> a piece of configuration (just like we'd do on a DT-based system)
Yes and maybe. The extra ACPI layer is inconvenient and more complex than a platform driver, as should be evident from the patch history and this discussion.
>
>> ACPI may be an ugly abomination, but it's what we're stuck with on x86 and it can only improve when more people get their hands on it.
> hmm, I'm planning to introduce DT to x86 (also patch up apu'2 coreboot
> do deliver a DT to the kernel) ... but just lacking time to get my hands
> dirty on this topic.
>
>> You might find it helpful to look at the coreboot source for the APU2 (src/mainboard/pcengines/apu2/gpio_ftns.h)
> In which version ?
>
>
> --mtx
>