Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Thu, Sep 30, 2021 at 1:40 AM Florian Eckert <fe@xxxxxxxxxx> wrote:

...

> > 2) driver seems supports the _ADR schema which you have used in ASL;
>
> This refers to the i2c-0, doesn't it?

Quite likely, but no guarantees. The number of the controller is
issued in order of enumeration which may be not deterministic. What is
real _ADR mapping here is that:

0 == physical port 0
1 == physical port 1
...
n == physical port n

> My mcp23s08 device is located at the i2c-0 on address 0x24.

...

> > 4) it's unclear what you did with ASL to get it loaded;
>
> On my development device I did a `iasl dsl/mcp23017.dsl`
> Of the following dsl
>
> $ cat dsl/mcp23017.dsl
> DefinitionBlock ("mcp23017.aml", "SSDT", 5, "", "MCP23017", 4)
> {
>      External (\_SB.PCI0.SBUS, DeviceObj)
>
>      Scope (\_SB.PCI0.SBUS)
>      {

According to the 2) above and as we learnt from the actual code this
missed one level of the devices, i.e. I2C port

Device (I2C0)
{
        Name (_ADR, Zero)

>          Device (GPIO)
>          {

>              Name (_HID, "PRP0001")
>              Name (_ADR, Zero)

This is against specification as I told you already. _CID/_HID may not
be together with _ADR.

>              Name (_CRS, ResourceTemplate () {
>                  I2cSerialBus (
>                      0x24,                   // Bus address
>                      ControllerInitiated,    // Don't care
>                      400000,                 // Fast mode (400 kHz)
>                      AddressingMode7Bit,     // 7-bit addressing
>                      "\\_SB.PCI0.SBUS",      // I2C host controller
>                      0                       // Must be 0
>                  )
>              })
>
>              Name (_DSD, Package () {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () {
>                      Package () { "compatible", Package () {
> "microchip,mcp23017" } },
>                  }
>              })
>          }
>      }
> }
>
> After that I copied this to my APU3 Target and executed
>   the following commands:
> mkdir /sys/kernel/config/acpi/table/mcp23017
>
> cat mcp23s08.aml > /sys/kernel/config/table/mcp23017

Yeah, but your I2C controller already been enumerated, so this won't
have any effect due to half-baked ACPI table in the firmware
(coreboot) which should be

Device (SBUS)
{
    Name (_ADR, 0x00140000)
    Device (PORT0)
    {
       Name (_ADR, Zero)
    }
    ...
    Device(PORTn)
    {
        Name (_ADR, n)
    }
}

...

> > 5) as Mika suggested, have you checked the kernel configuration?
>
> I have now switched on the suggested option
> CONFIG_ACPI_CUSTOM_METHOD=y
> CONFIG_ACPI_TABLE_UPGRADE=y
> CONFIG_CONFIGFS_FS=y
> CONFIG_ACPI_CONFIGFS=y
> CONFIG_ACPI_DEBUG=y
>
> But this did not solved my issue loading ssdt during runtime.

It won't as explained in 4) above.

> The output of the aml on the target:
>
> cat /sys/kernel/config/acpi/table/mcp23017/aml

Don't use `cat` against binary files. But we don't really need that.
It's the same as after iasl.

...

> My iasl version:
>
> iasl --version
> Illegal option: --
>
> Intel ACPI Component Architecture
> ASL+ Optimizing Compiler/Disassembler version 20181213

Please, update to something newer.

> Copyright (c) 2000 - 2018 Intel Corporation

...

Current summary:
 1) you still have issues with ASL
 2) there is boot ordering issues with the controller itself

> What else can I do?

So, the course of actions as I see is:
- fix the asl
- fix the coreboot to represent ports of the I2C controller
- switch to use initramfs method (or EFI)
- if possible, to unbind and bind back the i2c controller after AML is
loaded via configfs (it may be not, if it's occupied by some
peripheral devices / drivers that are crucial to system to work)

> The initrd option does not work with OpenWrt.

Why not? If indeed so (but I don't believe in this), OpenWRT has a bug
/ missed feature, which must be fixed there.

-- 
With Best Regards,
Andy Shevchenko