Search code examples
clinux-kernellinux-device-driverspiacpi

spi: ACPI kernel module autoloading broken


Kernel module autoloading for SPI client drivers that utilize ACPI SSDT to retrieve device configuration data isn't autoloading (executing probe function). Kernel module autoloading works fine with platform_driver (can successfully get properties from _DSD), but not with spi_driver. Read that spi_device_id must be defined and that the driver will only autoload using spi_device_id's. Are there any known workarounds? Is there any ACPI named objects associated with the spi_device_id struct?

ACPI SSDT Table

DefinitionBlock ("ili9488.aml", "SSDT", 5, "", "ILI9488", 0x1)
{
    Device (TFTD)
    {
        Name (_HID, "ILI9488")  // _HID: Hardware ID
        Name (_CID, "ILI9488")  // _CID: Compatible ID
        Name (_UID, One)        // _UID: Unique ID

        Name (_DSD, Package() { // Device Specific Data - used to return EC static resources (Defined in _CRS) to the driver.
            /*
             * Known as the Device Properties UUID
             * A generic UUID recognized by the ACPI subsystem in the Linux kernel which automatically
             * processes the data packages associated with it and makes data available to device driver
             * as "device properties".
             */
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package()
            {
                // Define compatible property (Not actually used CONFIG_OF not set)
                Package () { "compatible", Package () { "garosa,garosanvkr25fawd" } },
            }
        })

        Method (_CRS, 0, Serialized) {
            local0 = ResourceTemplate() { // Current Resource Settings (ACPI Resource to Buffer function)
                SpiSerialBus(
                    0,                    // Chip Select
                    PolarityLow,          // Chip Select is Active Low
                    FourWireMode,         // Full Duplex Mode (MISO Unused)
                    8,                    // 8-bits per word (1 byte)
                    ControllerInitiated,  // Slave Mode
                    32000000,             // 32 MHz
                    ClockPolarityLow,     // SPI Mode 0
                    ClockPhaseFirst,      // SPI Mode 0
                    "\\EC17.SPI0",        // SPI bus host controller name (unconfirmed)
                    0                     // Resource Index. Should be 0
                )
                // _SB.GPIO: GPIO bus host controller (unconfirmed)
                GpioIo (Exclusive, PullNone, 0, 0, IoRestrictionOutputOnly, "\\_SB.GPI0", 0, ResourceConsumer, , ) { 40 } // Pin 13
                GpioIo (Exclusive, PullUp,   0, 0, IoRestrictionOutputOnly, "\\_SB.GPI0", 0, ResourceConsumer, , ) { 45 } // Pin 15
            }
            Return(local0)
        }
    }
}

Snippet of driver

static int ili9488_probe(struct spi_device *spidev)
{
    struct device *device;
    const char *compatible_propery;

    device = &spidev->dev;
    device_property_read_string(device, "compatible", &compatible_propery);
    dev_warn(device, "compatible: %s", compatible_propery);

    return 0;
}

static void ili9488_remove(struct spi_device *spidev)
{
    dev_warn(&spidev->dev, "ENTER ili9488_remove function driver bound to ACPI table");
    return;
}

/* Device Tree Node to be compatible with */
static const struct of_device_id ili9488_of_match[] = {
    { .compatible = "garosa,garosanvkr25fawd" },
    {},
};

MODULE_DEVICE_TABLE(of, ili9488_of_match);

/*
 * Make compatible with ACPI SSDT via the
 * Compatible ID (_CID) name object
 */
static const struct acpi_device_id ili9488_acpi_match[] = {
    { "ILI9488", 0 },
    { }
};

MODULE_DEVICE_TABLE(acpi, ili9488_acpi_match);

static const struct spi_device_id ili9488_spi_devid[] = {
    { "ILI9488", 0 },
    { }
};

MODULE_DEVICE_TABLE(spi, ili9488_spi_devid);

/* Driver declaration */
static struct spi_driver ili9488_spi_driver = {
    .driver = {
        .name = "ili9488",
        .acpi_match_table = ili9488_acpi_match,
        .of_match_table = ili9488_of_match, // not necessary CONFIG_OF not even set
    },
    .id_table = ili9488_spi_devid,
    .probe    = ili9488_probe,
    .remove   = ili9488_remove,
};

module_spi_driver(ili9488_spi_driver);

After compiling both ACPI table and kernel module I tried loading with bellow results. If you add ACPI_PTR or remove of_match_table you don't see SPI driver warning.

$ mount -t configfs none /sys/kernel/config
$ modprobe acpi-configfs
$ mkdir -p /sys/kernel/config/acpi/table/ili9488
$ cat "ili9488.aml" > "/sys/kernel/config/acpi/table/ili9488/aml"
$ insmod ili9488.ko ; rmmod ili9488.ko 
SPI driver ili9488 has no spi_device_id for garosa,garosanvkr25fawd

Solution

  • ACPI table issues

    First problem with your table is a custom ACPI _HID and _CID. You can't create them out of the thin air. Yes, technically it's possible but only for your own use, no production. Moreover, your ID is created against ACPI specification. It's invalid, but Linux doesn't enforce this.

    Second issue, if you want to use compatible strings, you have to use PRP0001 ACPI _HID at the same time. Note, it doesn't matter if CONFIG_OF is y or n, Linux kernel has a glue layer in ACPI to parse this table. That's why the use of ACPI_PTR() and of_match_ptr() is highly discouraged.

    Third one, besides mentioned doubt about the GPIO controller path, you also have to be sure about the SPI controller path. To check what devices you really have (i.o.w. the devices that firmware provides as existing and running), run

    grep -H 15 /sys/bus/acpi/devices/*/status
    

    in the running (fresh enough) Linux OS.

    Ah, and actually fourth issue with the table (iasl compiler, if recent enough, must complain about) is that for the static data do not use Method(), use Name(), i.e. in case of _CRS.

    Driver issues

    First of all, SPI ID table should have an ID in lower case.

    Second, ACPI ID table is not needed as you don't have a real ACPI _HID for that device anyway. Just use compatible strings.

    Third, look at the existing examples of what you are trying to achieve, e.g., Ilitek 9486 driver code and in the proposed driver for the same hardware you have, i.e. Ilitek 9488.

    Also read ACPI Based Device Enumeration and _DSD Device Properties Related to GPIO. It will help you to understand a bit more about _DSD method in ACPI and how Linux interacts with it.

    Run-time concerns

    The warning you get with insmod is because your SPI ID table does not have "garosanvkr25fawd" in it. The SPI ID table should match to the vendor-less compatible strings.

    Hardware concerns

    According to the GitHub project page you provided, it seems that actually you missed the crucial detail — SPI is not the same as eSPI. They are quite different interfaces. That said, you may not connect an SPI display to an eSPI controller. It simply won't work.

    What you need is to understand how to enable General Purpose SPI (see Datasheet, Chapter 37).

    So, it seems this is a complete Embedded Controller that has its own firmware. Good luck with that! It will be a nice journey to the hardware-firmware-ACPI-OS details. (For now the ACPI tables is a least problem you have.)