linux-kernellinux-device-driveri2cpciirq

Linux PCI device interrupt not generated


I am currently refactoring a driver for the AMD Sensor Fusion Hub. The original driver can be found here. When sending commands to the device, the chip takes some time to process the request. I wasusing a hack using msleep to wait for the device to respond. However I'd like to cleanly implement IRQ handling. As a template I looked into the implementation of drivers/i2c/busses/i2c-amd-mp2-pci.c and came up with the following stub interrupt handling.

// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
/*
 * AMD Sensor Fusion Hub (SFH) PCIe driver
 *
 * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
 *          Nehal Bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
 *          Richard Neumann <mail@richard-neumann.de>
 */

#include <linux/bitops.h>
#include <linux/dma-mapping.h>
#include <linux/interrupt.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/pm_runtime.h>
#include <linux/types.h>

#include "amd-sfh-pci.h"

/**
 * amd_sfh_get_sensor_mask - Returns the sensors mask.
 * @pci_dev:    The Sensor Fusion Hub PCI device
 *
 * Returns an unsigned integer representing the bitmask to match
 * the sensors connected to the Sensor Fusion Hub.
 */
int amd_sfh_get_sensor_mask(struct pci_dev *pci_dev)
{
    int sensor_mask;
    struct amd_sfh_dev *sfh_dev = pci_get_drvdata(pci_dev);

    sensor_mask = readl(sfh_dev->mmio + AMD_SFH_SENSOR_MASK);
    /* Correct bit shift in firmware register */
    sensor_mask = sensor_mask >> 4;

    if (!sensor_mask)
        dev_err(&pci_dev->dev, "no sensors marked active on device\n");

    return sensor_mask;
}
EXPORT_SYMBOL_GPL(amd_sfh_get_sensor_mask);

/**
 * amd_sfh_start_sensor- Starts the respective sensor.
 * @pci_dev:    The Sensor Fusion Hub PCI device
 * @sensor_idx: The sensor's index
 * @dma_handle: The DMA handle
 */
void amd_sfh_start_sensor(struct pci_dev *pci_dev, enum sensor_idx sensor_idx,
                          dma_addr_t dma_handle)
{
    struct amd_sfh_dev *sfh_dev = pci_get_drvdata(pci_dev);
    union amd_sfh_cmd command;
    union amd_sfh_parm parameter;

    command.ul = 0;
    command.s.cmd_id = enable_sensor;
    command.s.period = PERIOD;
    command.s.sensor_id = sensor_idx;

    parameter.ul = 0;
    parameter.s.buffer_layout = 1;
    parameter.s.buffer_length = 16;

    writeq(dma_handle, sfh_dev->mmio + AMD_SFH_ADDR);
    writel(parameter.ul, sfh_dev->mmio + AMD_SFH_PARM);
    writel(command.ul, sfh_dev->mmio + AMD_SFH_CMD);
}
EXPORT_SYMBOL_GPL(amd_sfh_start_sensor);

/**
 * amd_sfh_stop_sensor- Stops the respective sensor.
 * @pci_dev:    The Sensor Fusion Hub PCI device
 * @sensor_idx: The sensor's index
 */
void amd_sfh_stop_sensor(struct pci_dev *pci_dev, enum sensor_idx sensor_idx)
{
    struct amd_sfh_dev *sfh_dev = pci_get_drvdata(pci_dev);
    union amd_sfh_cmd command;

    command.ul = 0;
    command.s.cmd_id = disable_sensor;
    command.s.period = 0;
    command.s.sensor_id = sensor_idx;

    writeq(0x0, sfh_dev->mmio + AMD_SFH_ADDR);
    writel(command.ul, sfh_dev->mmio + AMD_SFH_CMD);
}
EXPORT_SYMBOL_GPL(amd_sfh_stop_sensor);

/**
 * amd_sfh_stop_all_sensors- Stops all sensors on the SFH.
 * @pci_dev:    The Sensor Fusion Hub PCI device
 */
static void amd_sfh_stop_all_sensors(struct pci_dev *pci_dev)
{
    struct amd_sfh_dev *sfh_dev = pci_get_drvdata(pci_dev);
    union amd_sfh_cmd command;

    command.ul = 0;
    command.s.cmd_id = stop_all_sensors;
    command.s.period = 0;
    command.s.sensor_id = 0;

    writel(command.ul, sfh_dev->mmio + AMD_SFH_CMD);
}

/**
 * amd_sfh_irq_isr - IRQ handler
 * @irq:    The IRQ number received
 * @dev:    The underlying device
 */
static irqreturn_t amd_sfh_irq_isr(int irq, void *dev)
{
    struct amd_sfh_dev *sfh_dev = dev;

    dev_info(&sfh_dev->pci_dev->dev, "got IRQ: %d\n", irq);
    return IRQ_NONE;
}

/**
 * amd_sfh_pci_init - Initializes the PCI device
 * @sfh_dev:    The device data
 * @pci_dev:    The PCI device
 */
static int amd_sfh_pci_init(struct amd_sfh_dev *sfh_dev,
                            struct pci_dev *pci_dev)
{
    int rc;

    pci_set_drvdata(pci_dev, sfh_dev);
    rc = pcim_enable_device(pci_dev);

    if (rc)
        goto err_pci_enable;

    rc = pcim_iomap_regions(pci_dev, BIT(2), pci_name(pci_dev));

    if (rc)
        goto err_pci_enable;

    sfh_dev->pci_dev = pci_dev;
    sfh_dev->mmio = pcim_iomap_table(pci_dev)[2];
    pci_set_master(pci_dev);
    rc = pci_set_dma_mask(pci_dev, DMA_BIT_MASK(64));

    if (rc) {
        rc = pci_set_dma_mask(pci_dev, DMA_BIT_MASK(32));

        if (rc)
            goto err_dma_mask;
    }

    /* Set up intx irq */
    writel(0, sfh_dev->mmio + AMD_P2C_MSG_INTEN);
    pci_intx(pci_dev, 1);
    dev_info(&pci_dev->dev, "available interrupt: %d\n", pci_dev->irq);
    rc = devm_request_irq(&pci_dev->dev, pci_dev->irq, amd_sfh_irq_isr,
                  0, dev_name(&pci_dev->dev), sfh_dev);
    if (rc)
        dev_err(&pci_dev->dev, "Failure requesting irq %i: %d\n",
            pci_dev->irq, rc);

    return rc;

err_dma_mask:
    pci_clear_master(pci_dev);
err_pci_enable:
    return rc;
}

static int amd_sfh_pci_probe(struct pci_dev *pci_dev,
                 const struct pci_device_id *id)
{
    int rc;
    struct amd_sfh_dev *sfh_dev;

    sfh_dev = devm_kzalloc(&pci_dev->dev, sizeof(*sfh_dev), GFP_KERNEL);

    if (!sfh_dev)
        return -ENOMEM;

    rc = amd_sfh_pci_init(sfh_dev, pci_dev);

    if (rc)
        return rc;

    pm_runtime_set_autosuspend_delay(&pci_dev->dev, 1000);
    pm_runtime_use_autosuspend(&pci_dev->dev);
    pm_runtime_put_autosuspend(&pci_dev->dev);
    pm_runtime_allow(&pci_dev->dev);

    dev_info(&pci_dev->dev, "SFH device registered.\n");

    return 0;
}

static void amd_sfh_pci_remove(struct pci_dev *pci_dev)
{
    struct amd_sfh_dev *sfh_dev = pci_get_drvdata(pci_dev);

    amd_sfh_stop_all_sensors(sfh_dev->pci_dev);

    pm_runtime_forbid(&pci_dev->dev);
    pm_runtime_get_noresume(&pci_dev->dev);

    pci_intx(pci_dev, 0);
    pci_clear_master(pci_dev);
}

static const struct pci_device_id amd_sfh_pci_tbl[] = {
    {PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_SFH)},
    {0}
};
MODULE_DEVICE_TABLE(pci, amd_sfh_pci_tbl);

static struct pci_driver amd_sfh_pci_driver = {
    .name       = "amd-sfh-pci",
    .id_table   = amd_sfh_pci_tbl,
    .probe      = amd_sfh_pci_probe,
    .remove     = amd_sfh_pci_remove,
};
module_pci_driver(amd_sfh_pci_driver);

/**
 * amd_sfh_find_device - Returns the first best AMD SFH device.
 */
struct amd_sfh_dev *amd_sfh_find_device(void)
{
    struct device *dev;
    struct pci_dev *pci_dev;

    dev = driver_find_next_device(&amd_sfh_pci_driver.driver, NULL);

    if (!dev)
        return NULL;

    pci_dev = to_pci_dev(dev);
    return pci_get_drvdata(pci_dev);
}
EXPORT_SYMBOL_GPL(amd_sfh_find_device);

MODULE_DESCRIPTION("AMD(R) Sensor Fusion Hub PCI driver");
MODULE_AUTHOR("Shyam Sundar S K <Shyam-sundar.S-k@amd.com>");
MODULE_AUTHOR("Nehal Bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>");
MODULE_AUTHOR("Richard Neumann <mail@richard-neumann.de>");
MODULE_LICENSE("Dual BSD/GPL");

However the dev_info from amd_sfh_irq_isr is never logged, so I suspect that the IRQ is never triggered when writing to the device registers. What can be the reason for that and what can I do to correctly implement the IRQ handling?

PS dmesg output:

[ 2272.642762] amd-sfh-pci 0000:04:00.7: available interrupt: 56
[ 2272.642840] amd-sfh-pci 0000:04:00.7: SFH device registered.

Update The device seems to support MSI:

04:00.7 Non-VGA unclassified device [0000]: Advanced Micro Devices, Inc. [AMD] Raven/Raven2/Renoir Sensor Fusion Hub [1022:15e4]
    Subsystem: Hewlett-Packard Company Raven/Raven2/Renoir Sensor Fusion Hub [103c:8496]
    Flags: bus master, fast devsel, latency 0, IRQ 56
    Memory at fc800000 (32-bit, non-prefetchable) [size=1M]
    Memory at fcc8c000 (32-bit, non-prefetchable) [size=8K]
    Capabilities: [48] Vendor Specific Information: Len=08 <?>
    Capabilities: [50] Power Management version 3
    Capabilities: [64] Express Endpoint, MSI 00
    Capabilities: [a0] MSI: Enable- Count=1/2 Maskable- 64bit+
    Capabilities: [c0] MSI-X: Enable- Count=2 Masked-
    Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1 Len=010 <?>
    Kernel modules: amd_sfh_pci

But the interrupts are still not received / handled:

static void amd_sfh_debug(struct pci_dev *pci_dev)
{
    bool msi;

    msi = pci_dev_msi_enabled(pci_dev);
    pci_info(pci_dev, "MSI: %s\n", msi ? "true" : "false");
    pci_info(pci_dev, "No MSI: %s\n", pci_dev->no_msi ? "true" : "false");
}

/**
 * amd_sfh_handle_irq - Handles IRQs.
 * @irq:    The interrupt request to be handled
 * @dev:    The driver data
 *
 * Returns an appropriate IRQ return type.
 */
static irqreturn_t amd_sfh_handle_irq(int irq, void *dev)
{
    struct amd_sfh_dev *privdata = dev;

    pci_info(privdata->pci_dev, "got IRQ: %d\n", irq);
    return IRQ_NONE;
}

static int amd_sfh_setup_irq(struct amd_sfh_dev *privdata)
{
    int rc, vecs, irq;
    struct pci_dev *pci_dev = privdata->pci_dev;

    vecs = pci_alloc_irq_vectors(pci_dev, 1, 3, PCI_IRQ_ALL_TYPES);
    if (vecs < 0)
        return vecs;

    pci_info(pci_dev, "allocated %d IRQ vectors\n", vecs);

    for (irq = 0; irq < vecs; irq++) {
        pci_info(pci_dev, "requesting IRQ: %d\n", irq);
        rc = devm_request_irq(&pci_dev->dev,
                             pci_irq_vector(pci_dev, irq),
                             amd_sfh_handle_irq, 0, "sfh-irq",
                             privdata);
        if (rc) {
            pci_err(pci_dev, "failed to get IRQ\n");
            goto free_irq_vectors;
        }
    }

    return 0;

free_irq_vectors:
    pci_free_irq_vectors(pci_dev);
    return rc;
}

static int amd_sfh_pci_init(struct amd_sfh_dev *privdata,
                            struct pci_dev *pci_dev)
{
    int rc;

    pci_set_drvdata(pci_dev, privdata);

    rc = pcim_enable_device(pci_dev);
    if (rc)
        return rc;

    rc = pcim_iomap_regions(pci_dev, BIT(2), pci_name(pci_dev));
    if (rc)
        return rc;

    privdata->pci_dev = pci_dev;
    privdata->mmio = pcim_iomap_table(pci_dev)[2];
    pci_set_master(pci_dev);

    rc = pci_set_dma_mask(pci_dev, DMA_BIT_MASK(64));
    if (rc) {
        rc = pci_set_dma_mask(pci_dev, DMA_BIT_MASK(32));
        if (rc)
            goto clear_master;
    }

    /* Setup IRQ */
    amd_sfh_debug(pci_dev);

    rc = amd_sfh_setup_irq(privdata);
    if (rc)
        goto clear_master;

    amd_sfh_debug(pci_dev);
    /* End of IRQ setup */

    pci_info(pci_dev, "AMD Sensor Fusion Hub device initialized\n");

    return 0;

clear_master:
    pci_clear_master(pci_dev);
    return rc;
}

dmesg:

[    6.954524] amd-sfh-pci 0000:04:00.7: enabling device (0000 -> 0002)
[    6.954641] amd-sfh-pci 0000:04:00.7: MSI: false
[    6.954642] amd-sfh-pci 0000:04:00.7: No MSI: false
[    6.954791] amd-sfh-pci 0000:04:00.7: allocated 2 IRQ vectors
[    6.954792] amd-sfh-pci 0000:04:00.7: requesting IRQ: 0
[    6.954825] amd-sfh-pci 0000:04:00.7: requesting IRQ: 1
[    6.954860] amd-sfh-pci 0000:04:00.7: MSI: true
[    6.954861] amd-sfh-pci 0000:04:00.7: No MSI: false
[    6.954861] amd-sfh-pci 0000:04:00.7: AMD Sensor Fusion Hub device initialized
[    6.969691] amd-sfh-pci 0000:04:00.7: [Firmware Bug]: No sensors marked active!
[    6.971265] amd-sfh-pci 0000:04:00.7: sensor mask: 0x000001
[    7.548189] hid-generic 0018:03FE:0001.0001: hidraw0: I2C HID v0.01 Device [amd-sfh-accel] on 

The original documentation from AMD mentiones interrupts but is not too specific as to how they are being generated.


Solution

  • By try-and-error I found that I needed to set the P2C register 0x10690 to 1 in order to enable interrupts on the device. Whith this set, the device is flooding the driver with interrupts. I'm still figuring out how to make the device generate interrupts only on actual write events to the C2P registers.

    Okay, I found out, how: https://github.com/conqp/linux/blob/5ba797452a794100d65d103e8eb53f64ae14d1d0/drivers/hid/amd-sfh-hid/amd-sfh-pci.c#L301