Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 115 additions & 110 deletions drivers/bus/phytium_pio.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include <linux/cputypes.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/logic_pio.h>
#include <linux/module.h>
#include <linux/spinlock.h>
Expand All @@ -25,10 +28,8 @@
#define PHYT_PIO_DRV_VER "1.1.0"

static struct phytium_pio *ppio;
int phytium_pio_int_state;
int phytium_pio_clr_int;

static irqreturn_t phytium_pio_irq_handler(int irq, void *dev_id);
static int phytium_pio_int_state;
static int phytium_pio_clr_int;

bool check_cpu_type(void)
{
Expand All @@ -43,114 +44,97 @@ bool check_cpu_type(void)
}
EXPORT_SYMBOL(check_cpu_type);

void phytium_pio_clear_interrupt(u32 val)
static void phytium_pio_irq_noop(struct irq_data *d)
{
unsigned long flags;

if (!ppio)
return;
spin_lock_irqsave(&ppio->lock, flags);
writel(val, ppio->regbase + ppio->int_clear_reg);
readl(ppio->regbase + ppio->int_status_reg);
spin_unlock_irqrestore(&ppio->lock, flags);
}
EXPORT_SYMBOL(phytium_pio_clear_interrupt);

void phytium_pio_handle_interrupt_bit(unsigned int bit)
static struct irq_chip phytium_pio_irq_chip = {
.name = PHYT_PIO_DRIVER_NAME,
.irq_mask = phytium_pio_irq_noop,
.irq_unmask = phytium_pio_irq_noop,
.irq_ack = phytium_pio_irq_noop,
};
Comment on lines +51 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Keyboard IRQs cannot wake the system

When a Phytium i8042 port is configured as a wake source for suspend, the virtual IRQ chip has no .irq_set_wake callback, so enable_irq_wake() on the new per-source i8042 IRQs fails instead of configuring wake on the PIO parent. Phytium keyboard or touchpad input therefore cannot wake the system from suspend.

Add wake handling for the child IRQs that propagates wake configuration to the PIO parent IRQ.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 51-56:

**Keyboard IRQs cannot wake the system**

When a Phytium i8042 port is configured as a wake source for suspend, the virtual IRQ chip has no `.irq_set_wake` callback, so `enable_irq_wake()` on the new per-source i8042 IRQs fails instead of configuring wake on the PIO parent. Phytium keyboard or touchpad input therefore cannot wake the system from suspend.

Add wake handling for the child IRQs that propagates wake configuration to the PIO parent IRQ.


/* One Linux irq per latch bit, acknowledged once before the dispatch. */
static void phytium_pio_irq_dispatch(struct irq_desc *desc)
{
unsigned long flags;
struct phytium_pio *pio = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
unsigned long bits;
u32 pending, again;
unsigned int bit;

if (!ppio || bit >= 32)
return;
chained_irq_enter(chip, desc);

spin_lock_irqsave(&ppio->lock, flags);
ppio->handled_bits |= BIT(bit);
spin_unlock_irqrestore(&ppio->lock, flags);
}
EXPORT_SYMBOL(phytium_pio_handle_interrupt_bit);
pending = readl(pio->regbase + pio->int_status_reg);
if (!pending)
goto out;

void phytium_pio_unhandle_interrupt_bit(unsigned int bit)
{
unsigned long flags;
/* The latch is write-zero-to-clear. */
writel(~pending, pio->regbase + pio->int_clear_reg);
again = readl(pio->regbase + pio->int_status_reg);

if (!ppio || bit >= 32)
return;
bits = pending;
for_each_set_bit(bit, &bits, PHYTIUM_PIO_IRQ_NR)
Comment on lines +75 to +78

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Stuck parent interrupt keeps firing

When a latch bit remains asserted after the stuck-ack limit is exceeded, disable_irq_nosync() masks the parent, but chained_irq_exit() unconditionally unmasks it, so the stuck interrupt storm continues.

Handle a stuck source with a mechanism that remains effective after chained_irq_exit() unmasks the parent.

Also at drivers/bus/phytium_pio.c:88-93.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 75-78:

**Stuck parent interrupt keeps firing**

When a latch bit remains asserted after the stuck-ack limit is exceeded, `disable_irq_nosync()` masks the parent, but `chained_irq_exit()` unconditionally unmasks it, so the stuck interrupt storm continues.

Handle a stuck source with a mechanism that remains effective after `chained_irq_exit()` unmasks the parent.

Also at `drivers/bus/phytium_pio.c:88-93`.

generic_handle_domain_irq(pio->domain, bit);

spin_lock_irqsave(&ppio->lock, flags);
ppio->handled_bits &= ~BIT(bit);
spin_unlock_irqrestore(&ppio->lock, flags);
/*
* A source that is still asserted after its latch was cleared cannot
* be acknowledged; shut the line down instead of looping forever.
*/
if (again & pending && ++pio->stuck_acks > PHYTIUM_PIO_STUCK_LIMIT) {
pr_err("phytium-pio: latch bit 0x%x stuck, disabling irq %d\n",
again & pending, pio->irq);
disable_irq_nosync(pio->irq);
} else if (!(again & pending)) {
Comment on lines +85 to +89

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · One stuck source silences every device

When any latch source remains asserted for more than PHYTIUM_PIO_STUCK_LIMIT acknowledgements, the handler disables pio->irq after a latch remains asserted, but that is the single chained parent for all latch bits; one stuck source therefore stops delivery to unrelated keyboard, UART, and EC sources too.

Quarantine the stuck source without permanently disabling delivery for the other latch bits, or provide a recovery path that reenables the parent IRQ.

Also at drivers/bus/phytium_pio.c:339.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 85-89:

**One stuck source silences every device**

When any latch source remains asserted for more than `PHYTIUM_PIO_STUCK_LIMIT` acknowledgements, the handler disables `pio->irq` after a latch remains asserted, but that is the single chained parent for all latch bits; one stuck source therefore stops delivery to unrelated keyboard, UART, and EC sources too.

Quarantine the stuck source without permanently disabling delivery for the other latch bits, or provide a recovery path that reenables the parent IRQ.

Also at `drivers/bus/phytium_pio.c:339`.

pio->stuck_acks = 0;
}
Comment on lines +85 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Normal interrupts eventually disable PIO

When a latch bit remains asserted until its child IRQ handler services it, again is read before generic_handle_domain_irq() lets the child clear its source, so a normally serviced level-triggered interrupt increments stuck_acks. Since the counter resets only when that pre-dispatch read has no matching bits, ordinary interrupts accumulate until the parent IRQ is disabled, stopping all PIO children.

Check whether the source remains asserted after child dispatch, and reset the stuck counter when the child has cleared it.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 85-91:

**Normal interrupts eventually disable PIO**

When a latch bit remains asserted until its child IRQ handler services it, `again` is read before `generic_handle_domain_irq()` lets the child clear its source, so a normally serviced level-triggered interrupt increments `stuck_acks`. Since the counter resets only when that pre-dispatch read has no matching bits, ordinary interrupts accumulate until the parent IRQ is disabled, stopping all PIO children.

Check whether the source remains asserted after child dispatch, and reset the stuck counter when the child has cleared it.

out:
chained_irq_exit(chip, desc);
}
EXPORT_SYMBOL(phytium_pio_unhandle_interrupt_bit);

bool phytium_pio_clear_interrupt_bit(unsigned int bit)
static int phytium_pio_domain_map(struct irq_domain *d, unsigned int virq,
irq_hw_number_t hwirq)
{
unsigned long flags;
u32 pending;

if (!ppio || bit >= 32)
return false;

spin_lock_irqsave(&ppio->lock, flags);
pending = readl(ppio->regbase + ppio->int_status_reg);
if (pending & BIT(bit)) {
writel((u32)~BIT(bit), ppio->regbase + ppio->int_clear_reg);
readl(ppio->regbase + ppio->int_status_reg);
}
spin_unlock_irqrestore(&ppio->lock, flags);
irq_set_chip_and_handler(virq, &phytium_pio_irq_chip, handle_simple_irq);
irq_set_noprobe(virq);

return pending & BIT(bit);
return 0;
}
EXPORT_SYMBOL(phytium_pio_clear_interrupt_bit);

u32 phytium_pio_get_int_status(void)
static const struct irq_domain_ops phytium_pio_domain_ops = {
.map = phytium_pio_domain_map,
};

/**
* phytium_pio_irq_get - Linux irq of a latch bit
* @bit: latch bit, see PHYTIUM_PIO_*_INT
*
* The mapping is created on the first call.
*
* Return: the Linux irq, or a negative errno.
*/
int phytium_pio_irq_get(unsigned int bit)
{
unsigned long flags;
u32 ret = 0;
unsigned int virq;

if (!ppio)
return ret;
spin_lock_irqsave(&ppio->lock, flags);
ret = readl(ppio->regbase + ppio->int_status_reg);
spin_unlock_irqrestore(&ppio->lock, flags);
if (!ppio || !ppio->domain || bit >= PHYTIUM_PIO_IRQ_NR)
return -ENODEV;

return ret;
}
EXPORT_SYMBOL(phytium_pio_get_int_status);
virq = irq_create_mapping(ppio->domain, bit);

int phytium_pio_get_irq(void)
{
if (!ppio)
return -EINVAL;
return ppio->irq;
return virq ? virq : -ENOMEM;
}
EXPORT_SYMBOL(phytium_pio_get_irq);
EXPORT_SYMBOL_GPL(phytium_pio_irq_get);

/* Children live in the firmware window, not in the latch window. */
bool phytium_pio_is_lpc_mem(phys_addr_t addr)
{
return ppio && addr >= ppio->lpc_base &&
addr - ppio->lpc_base < PHYTIUM_LPC_IOMEM_SIZE;
return ppio && addr >= ppio->win_base &&
addr - ppio->win_base < PHYTIUM_PIO_LENGTH_LONG;
}
EXPORT_SYMBOL(phytium_pio_is_lpc_mem);

static irqreturn_t phytium_pio_irq_handler(int irq, void *dev_id)
{
struct phytium_pio *pio = dev_id;
unsigned long flags;
u32 clear_bits, pending;

spin_lock_irqsave(&pio->lock, flags);
pending = readl(pio->regbase + pio->int_status_reg);
clear_bits = pending & ~pio->handled_bits;

if (clear_bits) {
writel(~clear_bits, pio->regbase + pio->int_clear_reg);
readl(pio->regbase + pio->int_status_reg);
}
spin_unlock_irqrestore(&pio->lock, flags);

return IRQ_RETVAL(pending);
}

/*
* phytium_pio_in - input the data from the target I/O port
* @hostdata: pointer to the device information relevant to PIO controller
Expand Down Expand Up @@ -280,7 +264,6 @@ static int phytium_pio_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct logic_pio_hwaddr *range;
struct phytium_pio *pio;
resource_size_t reg_size = PHYTIUM_LPC_IOMEM_SIZE;
phys_addr_t reg_start;
struct resource *res;
int ret;
Expand All @@ -291,7 +274,7 @@ static int phytium_pio_probe(struct platform_device *pdev)
spin_lock_init(&pio->lock);

pio->membase = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR_OR_NULL(pio->membase))
if (IS_ERR(pio->membase))
return PTR_ERR(pio->membase);

pio->irq = platform_get_irq(pdev, 0);
Expand All @@ -300,21 +283,23 @@ static int phytium_pio_probe(struct platform_device *pdev)
return pio->irq;
}

/*
* The ACPI _CRS window only covers the short PIO aperture, while the
* interrupt latch registers live near the end of the long LPC window
* (base + 0x7ffffc0/0x7ffffc4). Map the long window starting at the
* firmware supplied LPC base (0x10000000 on D3000M, 0x20000000 on
* D2000/D3000): reading the latch through the short window can return
* unrelated device state such as UART_IIR.
*/
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
reg_start = res->start;
pio->lpc_base = reg_start;
pio->win_base = res->start;

pio->regbase = devm_ioremap(dev, reg_start, reg_size);
/* The latch window is a SoC property, not what resource 0 describes. */
if (cpu_is_ft_d2000()) {
reg_start = PHYTIUM_PIO_D2000_BASE;
} else if (is_pd2408()) {
reg_start = PHYTIUM_PIO_D3000M_BASE;
} else {
reg_start = res->start;
if (resource_size(res) < PHYTIUM_PIO_LENGTH_LONG)
Comment on lines +292 to +298

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · D3000 latch reads use firmware base

When a D3000 is not identified by either special-case predicate and its firmware resource 0 is shorter than the LPC window, on a D3000 that is neither classified as FT D2000 nor PD2408, phytium_pio_probe() selects res->start even when resource 0 is shorter than the LPC window. The latch is then accessed at the wrong physical address, so PIO child interrupts are not acknowledged or dispatched.

Select the 0x20000000 latch base for D3000 systems as well as D2000 systems, using a reliable SoC predicate.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 292-298:

**D3000 latch reads use firmware base**

When a D3000 is not identified by either special-case predicate and its firmware resource 0 is shorter than the LPC window, on a D3000 that is neither classified as FT D2000 nor PD2408, `phytium_pio_probe()` selects `res->start` even when resource 0 is shorter than the LPC window. The latch is then accessed at the wrong physical address, so PIO child interrupts are not acknowledged or dispatched.

Select the 0x20000000 latch base for D3000 systems as well as D2000 systems, using a reliable SoC predicate.

dev_warn(dev, "short window on an unknown SoC, the latch address may be wrong\n");
}

pio->regbase = devm_ioremap(dev, reg_start, PHYTIUM_PIO_LENGTH_LONG);
if (!pio->regbase)
return -ENOMEM;

Expand All @@ -334,24 +319,30 @@ static int phytium_pio_probe(struct platform_device *pdev)
range->hostdata = pio;
range->ops = &phytium_pio_ops;
pio->range = range;
ppio = pio;
ret = devm_request_irq(dev, pio->irq, phytium_pio_irq_handler,
IRQF_SHARED, dev_name(dev), pio);
if (ret) {
dev_err(dev, "Failed to request irq (%d)!\n", ret);
goto out;

pio->domain = irq_domain_create_linear(dev_fwnode(dev),
PHYTIUM_PIO_IRQ_NR,
&phytium_pio_domain_ops, pio);
if (!pio->domain) {
dev_err(dev, "Failed to create the latch irq domain\n");
return -ENOMEM;
}

ret = logic_pio_register_range(range);
if (ret) {
dev_err(dev, "Failed to register logic pio range (%d)!\n", ret);
goto out;
irq_domain_remove(pio->domain);
pio->domain = NULL;
return ret;
}

irq_set_chained_handler_and_data(pio->irq, phytium_pio_irq_dispatch, pio);

/* Children look the controller up through ppio. */
ppio = pio;
dev_set_drvdata(dev, pio);
out:
if (ret)
ppio = NULL;
return ret;

return 0;
}

static int phytium_pio_remove(struct platform_device *pdev)
Expand All @@ -363,6 +354,20 @@ static int phytium_pio_remove(struct platform_device *pdev)
if (!range)
return -EINVAL;

if (pio && pio->domain) {
int bit;

irq_set_chained_handler_and_data(pio->irq, NULL, NULL);
for (bit = 0; bit < PHYTIUM_PIO_IRQ_NR; bit++) {
unsigned int virq = irq_find_mapping(pio->domain, bit);

if (virq)
irq_dispose_mapping(virq);
}
irq_domain_remove(pio->domain);
Comment on lines +360 to +367

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · Removal races active latch dispatch

When the PIO device is removed while its parent interrupt is being dispatched, phytium_pio_remove() clears the chained handler and immediately disposes its mappings and domain without synchronizing the parent IRQ. An already-running phytium_pio_irq_dispatch() can still use pio and pio->domain as teardown proceeds, causing access to released state during removal.

Disable the parent IRQ and synchronize it after detaching the chained handler, before disposing mappings or releasing the PIO state.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 360-367:

**Removal races active latch dispatch**

When the PIO device is removed while its parent interrupt is being dispatched, `phytium_pio_remove()` clears the chained handler and immediately disposes its mappings and domain without synchronizing the parent IRQ. An already-running `phytium_pio_irq_dispatch()` can still use `pio` and `pio->domain` as teardown proceeds, causing access to released state during removal.

Disable the parent IRQ and synchronize it after detaching the chained handler, before disposing mappings or releasing the PIO state.

pio->domain = NULL;
Comment on lines +360 to +368

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · PIO removal leaves child IRQ users active

When the PIO ACPI device is removed while a child still has a requested IRQ, or while another child is obtaining one, phytium_pio_remove() detaches the parent handler and disposes the domain mappings without first stopping consumers that requested those IRQs. An active i8042, EC, or UART can therefore retain a registered action for a removed mapping, while a concurrent phytium_pio_irq_get() can use the domain as it is being removed; child interrupts or teardown then fail.

Ensure consumers are quiesced before removing the domain, and synchronize IRQ lookup with PIO teardown.

Also at drivers/bus/phytium_pio.c:122-124.

Prompt for AI agents
In `drivers/bus/phytium_pio.c` at lines 360-368:

**PIO removal leaves child IRQ users active**

When the PIO ACPI device is removed while a child still has a requested IRQ, or while another child is obtaining one, `phytium_pio_remove()` detaches the parent handler and disposes the domain mappings without first stopping consumers that requested those IRQs. An active i8042, EC, or UART can therefore retain a registered action for a removed mapping, while a concurrent `phytium_pio_irq_get()` can use the domain as it is being removed; child interrupts or teardown then fail.

Ensure consumers are quiesced before removing the domain, and synchronize IRQ lookup with PIO teardown.

Also at `drivers/bus/phytium_pio.c:122-124`.

}

if (ppio == pio)
ppio = NULL;
logic_pio_unregister_range(range);
Expand Down
28 changes: 14 additions & 14 deletions drivers/bus/phytium_pio.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,46 +9,46 @@
#define _PHYTIUM_PIO_H_

#include <linux/io.h>
#include <linux/irqdomain.h>
#include <linux/logic_pio.h>

#define PHYTIUM_PIO_LENGTH_LONG 0x8000000
#define FT3000_PIO_INT_STATE 0x7FFFFC4
#define FT3000_PIO_CLR_INT 0x7FFFFC0
#define FT2000_PIO_INT_STATE 0x7FFFFF4
#define FT2000_PIO_CLR_INT 0x7FFFFF0
#define LEGACY_ISA_SIZE 0x400
/* Base of the SoC window holding the interrupt latch registers. */
#define PHYTIUM_PIO_D2000_BASE 0x20000000
#define PHYTIUM_PIO_D3000M_BASE 0x10000000

/* Long LPC window holding the interrupt latch registers. */
#define PHYTIUM_LPC_IOMEM_SIZE 0x08000000
#define PHYTIUM_PIO_IRQ_NR 32
#define PHYTIUM_PIO_STUCK_LIMIT 1000

/* LPC interrupt latch bits shared by the legacy children. */
/* LPC interrupt latch bits, one Linux irq each. */
#define PHYTIUM_PIO_I8042_KBD_INT 1
#define PHYTIUM_PIO_UART_INT 4
#define PHYTIUM_PIO_EC_INT 11
#define PHYTIUM_PIO_I8042_AUX_INT 12

struct phytium_pio {
/* Firmware window: indirect PIO data accesses are decoded here. */
void __iomem *membase;
/* Long LPC window used for interrupt status/clear accesses. */
/* Long LPC window: holds the interrupt latch registers. */
void __iomem *regbase;
int irq;
spinlock_t lock;
u32 int_status_reg;
u32 int_clear_reg;
/* LPC latch bits owned by a registered child handler. */
u32 handled_bits;
phys_addr_t lpc_base;
struct irq_domain *domain;
u32 stuck_acks;
/* Physical start of the firmware window, see phytium_pio_is_lpc_mem(). */
phys_addr_t win_base;
struct logic_pio_hwaddr *range;
};

bool check_cpu_type(void);
int phytium_pio_get_irq(void);
void phytium_pio_handle_interrupt_bit(unsigned int bit);
void phytium_pio_unhandle_interrupt_bit(unsigned int bit);
bool phytium_pio_clear_interrupt_bit(unsigned int bit);
int phytium_pio_irq_get(unsigned int bit);
bool phytium_pio_is_lpc_mem(phys_addr_t addr);
void phytium_pio_clear_interrupt(u32 val);
u32 phytium_pio_get_int_status(void);

#endif

Expand Down
Loading
Loading