linux-kernel - Re: [PATCHv2 2/4] tps65912: irq: add interrupt controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 May 2011 20:56:10 +0100
From:	Jack Stone <jwjstone@...tmail.fm>
To:	Margarita Olaya <magi@...mlogic.co.uk>
CC:	linux-kernel@...r.kernel.org, Liam Girdwood <lrg@...mlogic.co.uk>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	sameo@...ux.intel.com
Subject: Re: [PATCHv2 2/4] tps65912: irq: add interrupt controller

Hi Margarita,

I'm no irq expert so the following might be wrong.

This comment seems to contradict the code - the comment states that all
interrupts are clear on read but the irq handler explicitly writes the
value back to the interrupt register.

Did you mean the comment to say that the irq handler explicitly clears
the interrupts it handles?

On 12/05/2011 19:43, Margarita Olaya wrote:
> +++ b/drivers/mfd/tps65912-irq.c
> +/*
> + * This is a threaded IRQ handler so can access I2C/SPI.  Since all
> + * interrupts are clear on read the IRQ line will be reasserted and
> + * the physical IRQ will be handled again if another interrupt is
> + * asserted while we run - in the normal course of events this is a
> + * rare occurrence so we save I2C/SPI reads.  We're also assuming that
> + * it's rare to get lots of interrupts firing simultaneously so try to
> + * minimise I/O.
> + */
> +static irqreturn_t tps65912_irq(int irq, void *irq_data)
> +{
> +	struct tps65912 *tps65912 = irq_data;
> +	u32 irq_sts;
> +	u32 irq_mask;
> +	u8 reg;
> +	int i;
> +
> +
> +	tps65912->read(tps65912, TPS65912_INT_STS, 1, &reg);
> +	irq_sts = reg;
> +	tps65912->read(tps65912, TPS65912_INT_STS2, 1, &reg);
> +	irq_sts |= reg << 8;
> +	tps65912->read(tps65912, TPS65912_INT_STS3, 1, &reg);
> +	irq_sts |= reg << 16;
> +	tps65912->read(tps65912, TPS65912_INT_STS4, 1, &reg);
> +	irq_sts |= reg << 24;
> +
> +	tps65912->read(tps65912, TPS65912_INT_MSK, 1, &reg);
> +	irq_mask = reg;
> +	tps65912->read(tps65912, TPS65912_INT_MSK2, 1, &reg);
> +	irq_mask |= reg << 8;
> +	tps65912->read(tps65912, TPS65912_INT_MSK3, 1, &reg);
> +	irq_mask |= reg << 16;
> +	tps65912->read(tps65912, TPS65912_INT_MSK4, 1, &reg);
> +	irq_mask |= reg << 24;
> +
> +	irq_sts &= ~irq_mask;
> +	if (!irq_sts)
> +		return IRQ_NONE;
> +
> +	for (i = 0; i < tps65912->irq_num; i++) {
> +		if (!(irq_sts & (1 << i)))
> +			continue;
> +
> +		handle_nested_irq(tps65912->irq_base + i);
> +	}
> +
> +	/* Write the STS register back to clear IRQs we handled */
> +	reg = irq_sts & 0xFF;
> +	irq_sts >>= 8;
> +	if (reg)
> +		tps65912->write(tps65912, TPS65912_INT_STS, 1, &reg);
> +	reg = irq_sts & 0xFF;
> +	irq_sts >>= 8;
> +	if (reg)
> +		tps65912->write(tps65912, TPS65912_INT_STS2, 1, &reg);
> +	reg = irq_sts & 0xFF;
> +	irq_sts >>= 8;
> +	if (reg)
> +		tps65912->write(tps65912, TPS65912_INT_STS3, 1, &reg);
> +	reg = irq_sts & 0xFF;
> +	if (reg)
> +		tps65912->write(tps65912, TPS65912_INT_STS4, 1, &reg);
> +
> +	return IRQ_HANDLED;
> +}

Thanks,

Jack
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/